Small improvement to MultiCFIteratorImpl (#13075)

Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/13075

The patch simplifies the iteration logic in `MultiCFIteratorImpl::{Advance,Populate}Iterator` a bit and adds some assertions to uniformly enforce the invariant that any iterators currently on the heap should be valid and have an OK status.

Reviewed By: jaykorean

Differential Revision: D64429566

fbshipit-source-id: 36bc22465285b670f859692a048e10f21df7da7a
This commit is contained in:
Levi Tamasi 2024-10-15 17:32:07 -07:00 committed by Facebook GitHub Bot
parent 2cb00c6921
commit 55de26580a
1 changed files with 45 additions and 29 deletions

View File

@ -211,17 +211,28 @@ class MultiCfIteratorImpl {
// 2. Make sure all others have iterated past the top iterator key slice // 2. Make sure all others have iterated past the top iterator key slice
// 3. Advance the top iterator, and add it back to the heap if valid // 3. Advance the top iterator, and add it back to the heap if valid
auto top = heap.top(); auto top = heap.top();
assert(top.iterator);
assert(top.iterator->Valid());
assert(top.iterator->status().ok());
heap.pop(); heap.pop();
if (!heap.empty()) {
while (!heap.empty()) {
auto current = heap.top(); auto current = heap.top();
assert(current.iterator); assert(current.iterator);
while (current.iterator->Valid() && assert(current.iterator->Valid());
comparator_->Compare(top.iterator->key(),
current.iterator->key()) == 0) {
assert(current.iterator->status().ok()); assert(current.iterator->status().ok());
if (comparator_->Compare(current.iterator->key(), top.iterator->key()) !=
0) {
break;
}
advance_func(current.iterator); advance_func(current.iterator);
if (current.iterator->Valid()) { if (current.iterator->Valid()) {
heap.replace_top(heap.top()); assert(current.iterator->status().ok());
heap.replace_top(current);
} else { } else {
considerStatus(current.iterator->status()); considerStatus(current.iterator->status());
if (!status_.ok()) { if (!status_.ok()) {
@ -231,12 +242,10 @@ class MultiCfIteratorImpl {
heap.pop(); heap.pop();
} }
} }
if (!heap.empty()) {
current = heap.top();
}
}
} }
advance_func(top.iterator); advance_func(top.iterator);
if (top.iterator->Valid()) { if (top.iterator->Valid()) {
assert(top.iterator->status().ok()); assert(top.iterator->status().ok());
heap.push(top); heap.push(top);
@ -264,30 +273,37 @@ class MultiCfIteratorImpl {
// populate the value/columns and attribute_groups from the list // populate the value/columns and attribute_groups from the list
// collected in step 1 and 2 and add all the iters back to the heap // collected in step 1 and 2 and add all the iters back to the heap
assert(!heap.empty()); assert(!heap.empty());
auto top = heap.top(); auto top = heap.top();
assert(top.iterator);
assert(top.iterator->Valid());
assert(top.iterator->status().ok());
heap.pop(); heap.pop();
autovector<MultiCfIteratorInfo> to_populate; autovector<MultiCfIteratorInfo> to_populate;
to_populate.push_back(top); to_populate.push_back(top);
if (!heap.empty()) {
while (!heap.empty()) {
auto current = heap.top(); auto current = heap.top();
assert(current.iterator); assert(current.iterator);
while (current.iterator->Valid() && assert(current.iterator->Valid());
comparator_->Compare(top.iterator->key(),
current.iterator->key()) == 0) {
assert(current.iterator->status().ok()); assert(current.iterator->status().ok());
to_populate.push_back(current);
heap.pop(); if (comparator_->Compare(current.iterator->key(), top.iterator->key()) !=
if (!heap.empty()) { 0) {
current = heap.top();
} else {
break; break;
} }
to_populate.push_back(current);
heap.pop();
} }
}
// Add the items back to the heap // Add the items back to the heap
for (auto& item : to_populate) { for (auto& item : to_populate) {
heap.push(item); heap.push(item);
} }
populate_func_(to_populate); populate_func_(to_populate);
} }
}; };