Skip to content

Commit c96da67

Browse files
committed
[BP] MB-56094/MB-58689: Update CM::numItems at ItemExpel
Quantity correctly updated in the CheckpointRemoval path, but we missed to update it when items are released in the Expel path. Change-Id: Ic05cbb3b9c98c6c9073b9b6f74fdfd6604cdcb74 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/200809 Tested-by: Paolo Cocchi <[email protected]> Reviewed-by: Jim Walker <[email protected]>
1 parent 8f2290e commit c96da67

File tree

2 files changed

+9
-7
lines changed

2 files changed

+9
-7
lines changed

engines/ep/src/checkpoint_manager.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,7 @@ CheckpointManager::expelUnreferencedCheckpointItems() {
707707
}
708708

709709
stats.itemsExpelledFromCheckpoints.fetch_add(numItemsExpelled);
710+
numItems.fetch_sub(numItemsExpelled);
710711

711712
// queueLock already released here, O(N) deallocation is lock-free
712713
const auto queuedItemsMemReleased = extractRes.deleteItems();

engines/ep/tests/module_tests/checkpoint_test.cc

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,9 @@ void CheckpointTest::testExpelCheckpointItems() {
23622362
* 1002 - 2nd item (key1) <<<<<<< persistenceCursor
23632363
* 1003 - 3rd item (key2)
23642364
*/
2365-
EXPECT_EQ(1 + itemCount, manager->getNumOpenChkItems());
2365+
const size_t expectedNumItemsPre = 1 + itemCount;
2366+
EXPECT_EQ(expectedNumItemsPre, manager->getNumOpenChkItems());
2367+
EXPECT_EQ(expectedNumItemsPre, manager->getNumItems());
23662368

23672369
const auto expelResult = manager->expelUnreferencedCheckpointItems();
23682370
EXPECT_EQ(2, expelResult.count);
@@ -2386,8 +2388,9 @@ void CheckpointTest::testExpelCheckpointItems() {
23862388
}
23872389

23882390
// 1 mutation removed from checkpoint
2389-
EXPECT_EQ(1 + (itemCount - expelResult.count),
2390-
manager->getNumOpenChkItems());
2391+
const size_t expectedNumItemsPost = 1 + (itemCount - expelResult.count);
2392+
EXPECT_EQ(expectedNumItemsPost, manager->getNumOpenChkItems());
2393+
EXPECT_EQ(expectedNumItemsPost, manager->getNumItems());
23912394

23922395
// Try to register a DCP replication cursor from 1001 - an expelled item.
23932396
std::string dcp_cursor1(DCP_CURSOR_PREFIX + std::to_string(1));
@@ -4771,8 +4774,7 @@ TEST_F(CDCCheckpointTest, DuplicateItemWhenPreviousExpelled) {
47714774
// [cs x x m(C):5) )
47724775
// ^
47734776
EXPECT_EQ(1, manager.getNumCheckpoints());
4774-
// Note: CM::numItems doesn't update at Expel in <=7.2.x (fixed in MB-56094)
4775-
EXPECT_EQ(4, manager.getNumItems()); // not updated by expel
4777+
EXPECT_EQ(2, manager.getNumItems()); // not updated by expel
47764778
EXPECT_EQ(2, manager.getNumOpenChkItems()); // updated by expel
47774779
EXPECT_EQ(5, manager.getHighSeqno());
47784780

@@ -4784,8 +4786,7 @@ TEST_F(CDCCheckpointTest, DuplicateItemWhenPreviousExpelled) {
47844786
// [cs x x m(C):5) ce] [cs m(A):6) )
47854787
// ^
47864788
EXPECT_EQ(2, manager.getNumCheckpoints());
4787-
// Note: CM::numItems doesn't update at Expel in <=7.2.x (fixed in MB-56094)
4788-
EXPECT_EQ(7, manager.getNumItems());
4789+
EXPECT_EQ(5, manager.getNumItems());
47894790
EXPECT_EQ(2, manager.getNumOpenChkItems());
47904791
EXPECT_EQ(6, manager.getHighSeqno());
47914792
EXPECT_GT(manager.getOpenCheckpointId(), ckptId);

0 commit comments

Comments
 (0)