Skip to content

Commit 4128ca7

Browse files
paolococchidaverigby
authored andcommitted
[BP] MB-57400: "estimated" items_remaining drops to 0 when cursor settles
[[backport to 7.1.x for maitanence patch]] "estimate" previously introduced in MB-57400. Here we introduce the invariant that "estimate" drops to 0 when the cursor reaches the end of checkpoint. That also fixes test issues like MB-57808. (cherry picked from commit e903cfe) Change-Id: Ieb65be994e279571e7c79d76a4b5df7f5042b9bc Reviewed-on: https://review.couchbase.org/c/kv_engine/+/195023 Well-Formed: Restriction Checker Reviewed-by: Paolo Cocchi <[email protected]> Tested-by: Dave Rigby <[email protected]>
1 parent 525edae commit 4128ca7

File tree

3 files changed

+15
-5
lines changed

3 files changed

+15
-5
lines changed

engines/ep/src/checkpoint_iterator.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,12 @@ class CheckpointIterator {
146146
return iter;
147147
}
148148

149-
private:
150149
/// Is the iterator currently pointing to the "end" element.
151150
bool isAtEnd() const {
152151
return (iter == container.get().end());
153152
}
154153

154+
private:
155155
/// Is the iterator currently pointing to the first element,
156156
bool isAtStart() const {
157157
return (iter == container.get().begin());

engines/ep/src/checkpoint_manager.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1269,8 +1269,16 @@ size_t CheckpointManager::getNumItemsForCursor(
12691269
bool accurate) const {
12701270
if (cursor && cursor->valid()) {
12711271
CheckpointList::const_iterator chkptIterator(cursor->getCheckpoint());
1272+
// Notes:
1273+
// - Estimate introduced in MB-57400 for avoiding expensive O(N) calls
1274+
// to cursor->getRemainingItemsCount()
1275+
// - MB-57808 improves that Estimate by introducing the invariant that
1276+
// the quantity drops to 0 when the cursor reaches the end of the
1277+
// checkpoint
12721278
size_t items = accurate ? cursor->getRemainingItemsCount()
1273-
: (*chkptIterator)->getNumItems();
1279+
: (std::next(cursor->getPos()).isAtEnd()
1280+
? 0
1281+
: (*chkptIterator)->getNumItems());
12741282

12751283
if (chkptIterator != checkpointList.end()) {
12761284
++chkptIterator;

engines/ep/tests/module_tests/checkpoint_test.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,15 +2763,17 @@ TEST_P(CheckpointTest, getNumItemsForCursor_Accurate_vs_Estimate) {
27632763
EXPECT_FALSE(res.moreAvailable);
27642764
EXPECT_EQ(3, items.size());
27652765

2766-
// Now we start to see a differnce, which can increase as the checkpoint
2767-
// grows.
2766+
// Note: The new Estimate is still precise in the case where the cursor has
2767+
// reached the end of the checkpoint.
27682768
EXPECT_EQ(0, manager->getNumItemsForCursor(cursor, true));
2769-
EXPECT_EQ(2, manager->getNumItemsForCursor(cursor, false));
2769+
EXPECT_EQ(0, manager->getNumItemsForCursor(cursor, false));
27702770

27712771
for (auto i : {1, 2}) {
27722772
EXPECT_TRUE(this->queueNewItem("b" + std::to_string(i)));
27732773
}
27742774

2775+
// Now we start to see a difference, which can increase as the checkpoint
2776+
// grows.
27752777
EXPECT_EQ(2, manager->getNumItemsForCursor(cursor, true));
27762778
EXPECT_EQ(4, manager->getNumItemsForCursor(cursor, false));
27772779

0 commit comments

Comments
 (0)