Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: fix _split_collections race with osr_reap #11748

Merged
merged 1 commit into from Nov 3, 2016

Conversation

liewegas
Copy link
Member

@liewegas liewegas commented Nov 2, 2016

The SharedBlobSet may not yet be empty at split time because previous
transactions may not have been reaped, leaving some Blobs still alive
even after the cache refs are cleared. Drop them explicitly here (they
will go away shortly).

Signed-off-by: Sage Weil sage@redhat.com

@liewegas
Copy link
Member Author

liewegas commented Nov 3, 2016

@ifed01 mind taking a look?

// shortly by _osr_reap_done, but it's awkward to block for that (and
// a waste of time). Instead, explicitly remove them from the shared blob
// map.
c->shared_blob_set.violently_clear();
assert(c->shared_blob_set.empty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this assert any more.

// shortly by _osr_reap_done, but it's awkward to block for that (and
// a waste of time). Instead, explicitly remove them from the shared blob
// map.
c->shared_blob_set.violently_clear();
assert(c->shared_blob_set.empty());
assert(d->shared_blob_set.empty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this makes much sense but is it possible that destination collection struggles (or will struggle one day) with the same issue? Does it worth to clear its shared_blob_set the same way? Just in case...

@liewegas
Copy link
Member Author

liewegas commented Nov 3, 2016 via email

The SharedBlobSet may not yet be empty at split time because previous
transactions may not have been reaped, leaving some Blobs still alive
even after the cache refs are cleared.  Drop them explicitly here (they
will go away shortly).

Signed-off-by: Sage Weil <sage@redhat.com>
@ifed01 ifed01 merged commit 8e1e51a into ceph:master Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants