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: prevent extent merging across shard boundaries #11216

Merged
merged 2 commits into from Sep 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/os/bluestore/BlueStore.cc
Expand Up @@ -1888,6 +1888,20 @@ int BlueStore::ExtentMap::compress_extent_map(uint64_t offset, uint64_t length)
if (p != extent_map.begin()) {
--p; // start to the left of offset
}

// identify the *next* shard
auto pshard = shards.begin();
while (pshard != shards.end() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Really should assert that p != end(). Not sure if seek_lextent really guarantees it. But if it doesn't things will really go south when you do n = p+1 later :(

Choose a reason for hiding this comment

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

seek_lextent doesn't guarantee that and that's why all the caller of seek_lextent() has a end check except this.
We should add that check or assert here I agree.

p->logical_offset >= pshard->offset) {
++pshard;
}
uint64_t shard_end;
if (pshard != shards.end()) {
shard_end = pshard->offset;
} else {
shard_end = OBJECT_MAX_SIZE;
}

auto n = p;
for (++n; n != extent_map.end(); p = n++) {
if (n->logical_offset > offset + length) {
Expand All @@ -1896,14 +1910,26 @@ int BlueStore::ExtentMap::compress_extent_map(uint64_t offset, uint64_t length)
while (n != extent_map.end() &&
p->logical_offset + p->length == n->logical_offset &&
p->blob == n->blob &&
p->blob_offset + p->length == n->blob_offset) {
p->blob_offset + p->length == n->blob_offset &&
n->logical_offset < shard_end) {
dout(20) << __func__ << " 0x" << std::hex << offset << "~" << length
<< " next shard 0x" << shard_end << std::dec
<< " merging " << *p << " and " << *n << dendl;
p->length += n->length;
rm(n++);
++removed;
}
if (n == extent_map.end()) {
break;
}
if (n->logical_offset >= shard_end) {
++pshard;
if (pshard != shards.end()) {
shard_end = pshard->offset;
} else {
shard_end = OBJECT_MAX_SIZE;
}
}
}
return removed;
}
Expand Down
49 changes: 49 additions & 0 deletions src/test/objectstore/store_test.cc
Expand Up @@ -3951,6 +3951,55 @@ TEST_P(StoreTest, SyntheticMatrixSharding) {
do_matrix(m, store);
}

TEST_P(StoreTest, ZipperPatternSharded) {
if(string(GetParam()) != "bluestore")
return;
g_conf->set_val("bluestore_min_alloc_size", "4096");
g_ceph_context->_conf->apply_changes(NULL);
int r = store->umount();
ASSERT_EQ(r, 0);
r = store->mount(); //to force min_alloc_size update
ASSERT_EQ(r, 0);

ObjectStore::Sequencer osr("test");
coll_t cid;
ghobject_t a(hobject_t(sobject_t("Object 1", CEPH_NOSNAP)));
{
ObjectStore::Transaction t;
t.create_collection(cid, 0);
cerr << "Creating collection " << cid << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
bufferlist bl;
int len = 4096;
bufferptr bp(len);
bp.zero();
bl.append(bp);
for (int i=0; i<1000; ++i) {
ObjectStore::Transaction t;
t.write(cid, a, i*2*len, len, bl, 0);
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
for (int i=0; i<1000; ++i) {
ObjectStore::Transaction t;
t.write(cid, a, i*2*len + 1, len, bl, 0);
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
{
ObjectStore::Transaction t;
t.remove(cid, a);
t.remove_collection(cid);
cerr << "Cleaning" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
g_conf->set_val("bluestore_min_alloc_size", "0");
g_ceph_context->_conf->apply_changes(NULL);
}

TEST_P(StoreTest, SyntheticMatrixCsumAlgorithm) {
if (string(GetParam()) != "bluestore")
return;
Expand Down