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: partial reshard support #13162

Merged
merged 20 commits into from Feb 7, 2017

Conversation

liewegas
Copy link
Member

Reshard only a subset of the extent map. If a single extent gets big, we will reshard only
it. If one gets small, we will combine with the smaller of our two neighbors.

@liewegas liewegas changed the title os/bluestore: partial reshard support DNM: os/bluestore: partial reshard support Jan 27, 2017
@liewegas liewegas force-pushed the wip-bluestore-reshard branch 2 times, most recently from a3e3ffc to 7230c6f Compare January 27, 2017 22:25

// unspan spanning blobs
if (e.blob->is_spanning()) {
spanning_blob_map.erase(e.blob->id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simply call spanning_blob_map.clear() after the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

not anymore.. we only want to unspan the blobs within the given range

if (len > cct->_conf->bluestore_extent_map_shard_max_size) {
// we are big; reshard ourselves
request_reshard(p->offset, endoff);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that correct to return here and below? Shouldn't we process all dirty shards as this can expand need_reshard range?

b->id = bid++;
spanning_blob_map[b->id] = b;
dout(20) << __func__ << " adding spanning " << *b << dendl;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

else -> else if(e->blob->is_spanning())

unsigned n = sv.size();
si_end = si_begin + new_shard_info.size();
for (unsigned i = si_begin; i < si_end; ++i) {
shards[i].offset = sv[i].offset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we need offset field in Shard at all, isn't one in shard_info enough?

@@ -633,14 +662,31 @@ class BlueStore : public ObjectStore,
struct Shard {
bluestore_onode_t::shard_info *shard_info;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to set to nullptr

si_end < shards.size())) {
// we resharded a partial range; we must produce at least one output
// shard
new_shard_info.emplace_back(bluestore_onode_t::shard_info());
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to implement shard_info(offset) ctor and apply it here and throughout?

Signed-off-by: Sage Weil <sage@redhat.com>
This gives us a single point of control for reshard, and tells us
which range we care about at the same time.

Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
…ently

Split ourselves, or merge with our immediate predecessor or
successor.

Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
This ensure we mop up shards past EOF instead of encoding them
empty and confusing future code (that, say, assumes no shards past
EOF).

Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas liewegas changed the title DNM: os/bluestore: partial reshard support os/bluestore: partial reshard support Feb 2, 2017
@liewegas
Copy link
Member Author

liewegas commented Feb 2, 2017

Updated, with several bugs fixed. And rebased now that use_tracker is merged. Please take a look!

Signed-off-by: Sage Weil <sage@redhat.com>
It's much more helpful this way.

Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Only reshard extents in the requested range.

Note that the strategy for unspanning blobs changed; we now span or
unspan specific blobs at the end based on our reshard result instead
of unspanning all at the start.  This keeps the spanning id stable,
which is important because it may be referenced from another that
we aren't even looking at.

Also note that this requires a bit of restructuring: an encode_some may
hit a spanning shard, *requiring* us to reshard, which means we should
always conduct the initial pass through update to discover other reshard
requirements, even if we already know some resharding will be needed.

Signed-off-by: Sage Weil <sage@redhat.com>
We can do this with a single allocation with a simple vector<>.

Signed-off-by: Sage Weil <sage@redhat.com>
This is mirrored in shard_info, which we have a pointer to.

Note that shard_info is also a bit redundant as we can also look it up in
the onode vector in the same position, but it makes it more awkward to use
iterators.  Something to consider later to save memory.

Signed-off-by: Sage Weil <sage@redhat.com>
// we are small; combine with a neighbor
if (p == shards.begin() && endoff == OBJECT_MAX_SIZE) {
// we are an only shard
request_reshard(0, OBJECT_MAX_SIZE);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll exist the loop at this iteration and return the function anyway hence just a noice

} else if (p == shards.begin()) {
// combine with next shard
request_reshard(p->offset, endoff + 1);
} else if (endoff == OBJECT_MAX_SIZE) {
// combine with previous shard
request_reshard(prev_p->offset, endoff);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@liewegas
Copy link
Member Author

liewegas commented Feb 7, 2017

@liewegas liewegas merged commit d62a494 into ceph:master Feb 7, 2017
@liewegas liewegas deleted the wip-bluestore-reshard branch February 7, 2017 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants