Merge pull request #11771 from liewegas/wip-bluestore-collection-end

os/bluestore: fix collection_list end bound off-by-one

Reviewed-by: Sage Weil <sage@redhat.com>
This commit is contained in:
Sage Weil 2016-11-04 08:26:17 -05:00 committed by GitHub
commit 648ea38d49
3 changed files with 56 additions and 3 deletions

View File

@ -5672,7 +5672,7 @@ int BlueStore::_collection_list(
}
dout(20) << __func__ << " pend " << pretty_binary_string(pend) << dendl;
while (true) {
if (!it->valid() || it->key() > pend) {
if (!it->valid() || it->key() >= pend) {
if (!it->valid())
dout(20) << __func__ << " iterator not valid (end of db?)" << dendl;
else
@ -5691,7 +5691,7 @@ int BlueStore::_collection_list(
}
break;
}
dout(20) << __func__ << " key " << pretty_binary_string(it->key()) << dendl;
dout(30) << __func__ << " key " << pretty_binary_string(it->key()) << dendl;
if (is_extent_shard_key(it->key())) {
it->next();
continue;
@ -5699,6 +5699,7 @@ int BlueStore::_collection_list(
ghobject_t oid;
int r = get_key_object(it->key(), &oid);
assert(r == 0);
dout(20) << __func__ << " oid " << oid << " end " << end << dendl;
if (ls->size() >= (unsigned)max) {
dout(20) << __func__ << " reached max " << max << dendl;
*pnext = oid;

View File

@ -1498,7 +1498,7 @@ int KStore::_collection_list(
}
dout(20) << __func__ << " pend " << pretty_binary_string(pend) << dendl;
while (true) {
if (!it->valid() || it->key() > pend) {
if (!it->valid() || it->key() >= pend) {
if (!it->valid())
dout(20) << __func__ << " iterator not valid (end of db?)" << dendl;
else

View File

@ -2367,6 +2367,58 @@ TEST_P(StoreTest, SimpleListTest) {
}
}
TEST_P(StoreTest, ListEndTest) {
ObjectStore::Sequencer osr("test");
int r;
coll_t cid(spg_t(pg_t(0, 1), shard_id_t(1)));
{
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);
}
set<ghobject_t, ghobject_t::BitwiseComparator> all;
{
ObjectStore::Transaction t;
for (int i=0; i<200; ++i) {
string name("object_");
name += stringify(i);
ghobject_t hoid(hobject_t(sobject_t(name, CEPH_NOSNAP)),
ghobject_t::NO_GEN, shard_id_t(1));
hoid.hobj.pool = 1;
all.insert(hoid);
t.touch(cid, hoid);
cerr << "Creating object " << hoid << std::endl;
}
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
{
ghobject_t end(hobject_t(sobject_t("object_100", CEPH_NOSNAP)),
ghobject_t::NO_GEN, shard_id_t(1));
end.hobj.pool = 1;
vector<ghobject_t> objects;
ghobject_t next;
int r = store->collection_list(cid, ghobject_t(), end,
true, 500,
&objects, &next);
ASSERT_EQ(r, 0);
for (auto &p : objects) {
ASSERT_NE(p, end);
}
}
{
ObjectStore::Transaction t;
for (set<ghobject_t, ghobject_t::BitwiseComparator>::iterator p = all.begin(); p != all.end(); ++p)
t.remove(cid, *p);
t.remove_collection(cid);
cerr << "Cleaning" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
}
TEST_P(StoreTest, Sort) {
{
hobject_t a(sobject_t("a", CEPH_NOSNAP));