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

Jump to bottom

bluestore: wrap blob id when it reaches maximum value of int16_t #15654

Merged
merged 1 commit into from Jun 15, 2017
Merged

bluestore: wrap blob id when it reaches maximum value of int16_t #15654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 27 additions & 8 deletions src/os/bluestore/BlueStore.cc
View file
Open in desktop
Expand Up @@ -36,6 +36,8 @@
#define dout_context cct
#define dout_subsys ceph_subsys_bluestore

using bid_t = decltype(BlueStore::Blob::id);

// bluestore_cache_onode
MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::Onode, bluestore_onode,
bluestore_cache_onode);
Expand Down Expand Up @@ -661,7 +663,7 @@ void BlueStore::GarbageCollector::process_protrusive_extents(
}
bExit = it == bi.last_lextent;
++it;
} while(!bExit);
} while (!bExit);
}
expected_for_release += blob_expected_for_release;
expected_allocations += bi.expected_allocations;
Expand Down Expand Up @@ -2019,6 +2021,28 @@ void BlueStore::ExtentMap::update(KeyValueDB::Transaction t,
}
}

bid_t BlueStore::ExtentMap::allocate_spanning_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.

nit, make this method a const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The return variable is a right value, should we make it const? As with const it causes the warning "warning: type qualifiers ignored on function return type".

Copy link
Member

Choose a reason for hiding this comment

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

The return variable is a right value, should we make it const?

I guess what does kefu mean is to make the method itself const, not the return value...

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, to be specific:

bid_t BlueStore::ExtentMap::allocate_spanning_blob_id() const

{
if (spanning_blob_map.empty())
return 0;
bid_t bid = spanning_blob_map.rbegin()->first + 1;
// bid is valid and available.
if (bid >= 0)
return bid;
// Find next unused bid;
bid = rand() % (numeric_limits<bid_t>::max() + 1);
const auto begin_bid = bid;
do {
if (!spanning_blob_map.count(bid))
return bid;
else {
bid++;
if (bid < 0) bid = 0;
}
} while (bid != begin_bid);
assert(0 == "no available blob id");
Copy link
Contributor

@tchaikov tchaikov Jun 14, 2017 •
edited

Choose a reason for hiding this comment

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

i am not sure this compiles... it's supposed to return something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I studied this from current codes. I think it makes assert understood easily.
https://github.com/ceph/ceph/blob/master/src/os/bluestore/BlueStore.cc#L740
https://github.com/ceph/ceph/blob/master/src/os/bluestore/BlueStore.cc#L947

Copy link
Contributor

Choose a reason for hiding this comment

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

@lixiaoy1 i am not questioning the assert() statement. i am wondering if a function is not returning void then the compiler would expect it to return a variable of the returning type.

}

void BlueStore::ExtentMap::reshard(
KeyValueDB *db,
KeyValueDB::Transaction t)
Expand Down Expand Up @@ -2208,12 +2232,6 @@ void BlueStore::ExtentMap::reshard(
} else {
shard_end = sp->offset;
}
int bid;
if (spanning_blob_map.empty()) {
bid = 0;
} else {
bid = spanning_blob_map.rbegin()->first + 1;
}
Extent dummy(needs_reshard_begin);
for (auto e = extent_map.lower_bound(dummy); e != extent_map.end(); ++e) {
if (e->logical_offset >= needs_reshard_end) {
Expand Down Expand Up @@ -2267,7 +2285,8 @@ void BlueStore::ExtentMap::reshard(
must_span = true;
}
if (must_span) {
b->id = bid++;
auto bid = allocate_spanning_blob_id();
b->id = bid;
spanning_blob_map[b->id] = b;
dout(20) << __func__ << " adding spanning " << *b << dendl;
}
Expand Down
1 change: 1 addition & 0 deletions src/os/bluestore/BlueStore.h
View file
Open in desktop
Expand Up @@ -776,6 +776,7 @@ class BlueStore : public ObjectStore,
}

void update(KeyValueDB::Transaction t, bool force);
decltype(BlueStore::Blob::id) allocate_spanning_blob_id();
void reshard(
KeyValueDB *db,
KeyValueDB::Transaction t);
Expand Down