Skip to content

Commit

Permalink
MB-48531: Remove CheckpointVisitor
Browse files Browse the repository at this point in the history
Unused now, all logic moved to ClosedUnrefCheckpointRemoverTask in
previous patches.

Change-Id: I5fe0508bd2cf1660989f6df4f82d72dadceeafcf
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/163268
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
  • Loading branch information
paolococchi committed Oct 12, 2021
1 parent 385754b commit 44a18e9
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 257 deletions.
1 change: 0 additions & 1 deletion engines/ep/CMakeLists.txt
Expand Up @@ -300,7 +300,6 @@ ADD_LIBRARY(ep_objs OBJECT
src/checkpoint_config.cc
src/checkpoint_manager.cc
src/checkpoint_remover.cc
src/checkpoint_visitor.cc
src/conflict_resolution.cc
src/conn_notifier.cc
src/connhandler.cc
Expand Down
121 changes: 0 additions & 121 deletions engines/ep/src/checkpoint_visitor.cc

This file was deleted.

75 changes: 0 additions & 75 deletions engines/ep/src/checkpoint_visitor.h

This file was deleted.

70 changes: 16 additions & 54 deletions engines/ep/tests/module_tests/checkpoint_remover_test.cc
Expand Up @@ -19,7 +19,6 @@
#include "checkpoint_manager.h"
#include "checkpoint_remover.h"
#include "checkpoint_utils.h"
#include "checkpoint_visitor.h"
#include "collections/vbucket_manifest_handles.h"
#include "dcp/response.h"
#include "test_helpers.h"
Expand Down Expand Up @@ -60,39 +59,6 @@ TEST_F(CheckpointRemoverEPTest, GetVBucketsSortedByChkMgrMem) {
}
}

/**
* Check that CheckpointVisitor orders vbuckets to visit by "highest checkpoint
* mem-usage" order.
*/
TEST_F(CheckpointRemoverEPTest, CheckpointVisitorVBucketOrder) {
std::deque<Vbid> vbuckets;
for (uint16_t i = 0; i < 3; i++) {
setVBucketStateAndRunPersistTask(Vbid(i), vbucket_state_active);
for (uint16_t j = 0; j < i; j++) {
std::string doc_key =
"key_" + std::to_string(i) + "_" + std::to_string(j);
store_item(Vbid(i), makeStoredDocKey(doc_key), "value");
}
vbuckets.emplace_back(vbid);
}

std::atomic<bool> stateFinalizer = true;
auto visitor = CheckpointVisitor(
store, engine->getEpStats(), stateFinalizer, 1 /*memToClear*/);
auto comparator = visitor.getVBucketComparator();
std::sort(vbuckets.begin(), vbuckets.end(), comparator);

ASSERT_EQ(3, vbuckets.size());
for (size_t i = 1; i < vbuckets.size(); i++) {
auto thisVbid = vbuckets[i];
auto prevVbid = vbuckets[i - 1];
// This vBucket should have a greater memory usage than the one previous
// to it in the map
ASSERT_GE(store->getVBucket(thisVbid)->getChkMgrMemUsage(),
store->getVBucket(prevVbid)->getChkMgrMemUsage());
}
}

#ifndef WIN32
/**
* Check that the CheckpointManager memory usage calculation is correct and
Expand Down Expand Up @@ -432,22 +398,18 @@ void CheckpointRemoverEPTest::testExpellingOccursBeforeCursorDropping(
// Testing ItemExpel
std::vector<queued_item> items;
manager->getNextItemsForCursor(cursor.get(), items);
} // Testing CursorDrop otherwise

const auto& remover =
std::make_shared<ClosedUnrefCheckpointRemoverTask>(
engine.get(),
engine->getEpStats(),
engine->getConfiguration().getChkRemoverStime());
remover->run();
getCkptDestroyerTask(vbid).run();
} else {
// Testing CursorDrop
std::atomic<bool> stateFinalizer = false;
auto visitor = CheckpointVisitor(
store, engine->getEpStats(), stateFinalizer, memToClear);
visitor.visitBucket(vb);
getCkptDestroyerTask(vbid).run();
const auto remover = std::make_shared<ClosedUnrefCheckpointRemoverTask>(
engine.get(),
engine->getEpStats(),
engine->getConfiguration().getChkRemoverStime());
remover->run();
getCkptDestroyerTask(vbid).run();

if (moveCursor) {
EXPECT_EQ(stats.getNumCheckpoints(), inititalNumCheckpoints);
} else {
EXPECT_LT(stats.getNumCheckpoints(), inititalNumCheckpoints);
}

Expand Down Expand Up @@ -961,12 +923,12 @@ TEST_F(CheckpointRemoverEPTest, CheckpointRemovalWithoutCursorDrop) {
ASSERT_EQ(0, engine->getEpStats().itemsRemovedFromCheckpoints);
ASSERT_EQ(0, engine->getEpStats().cursorsDropped);

const auto memToClear = store->getRequiredCheckpointMemoryReduction();
EXPECT_GT(memToClear, 0);
std::atomic<bool> stateFinalizer = false;
auto visitor = CheckpointVisitor(
store, engine->getEpStats(), stateFinalizer, memToClear);
visitor.visitBucket(vb);
ASSERT_GT(store->getRequiredCheckpointMemoryReduction(), 0);
const auto remover = std::make_shared<ClosedUnrefCheckpointRemoverTask>(
engine.get(),
engine->getEpStats(),
engine->getConfiguration().getChkRemoverStime());
remover->run();
getCkptDestroyerTask(vbid).run();

EXPECT_EQ(0, store->getRequiredCheckpointMemoryReduction());
Expand Down
7 changes: 2 additions & 5 deletions executor/fake_executorpool.h
Expand Up @@ -159,11 +159,8 @@ class CheckedExecutor : public CB3ExecutorThread {
} else if (getTaskName() ==
"Removing closed unreferenced checkpoints from memory") {
checker = [=](bool taskRescheduled) {
// This task _may_ schedule or wake 1 or 2 subsequent tasks.
// 1. A CheckpointVisitor, scanning all vbuckets for
// Checkpoints to remove
// 2. A CheckpointDestroyer, to destroy checkpoints made
// unreferenced by cursor dropping
// This task _may_ schedule or wake a CheckpointDestroyer, to
// destroy checkpoints made unreferenced by cursor dropping
this->oneExecutes(taskRescheduled, /*min*/ 0, /*max*/ 2);
};
} else if (getTaskName() == "Paging out items." ||
Expand Down
1 change: 0 additions & 1 deletion executor/tasks.def.h
Expand Up @@ -86,7 +86,6 @@ TASK(ActiveStreamCheckpointProcessorTask, NONIO_TASK_IDX, 3)
TASK(ConnNotifierCallback, NONIO_TASK_IDX, 5)
TASK(CheckpointDestroyerTask, NONIO_TASK_IDX, 6)
TASK(ClosedUnrefCheckpointRemoverTask, NONIO_TASK_IDX, 6)
TASK(ClosedUnrefCheckpointRemoverVisitorTask, NONIO_TASK_IDX, 6)
TASK(VBucketMemoryDeletionTask, NONIO_TASK_IDX, 6)
TASK(StatCheckpointTask, NONIO_TASK_IDX, 7)
TASK(StatDCPTask, NONIO_TASK_IDX, 7)
Expand Down

0 comments on commit 44a18e9

Please sign in to comment.