MB-38377: Make test run for all backends

The test RollbackCompletionCallbackStateAfterCompletionCallbackFailure tests that the completion callback is rolled back on compaction failure. It does not test anything KVStore specific and can be run for any backend. Change-Id: I9b3eacc80ade3b87e6aec6cd401676779e00810f Reviewed-on: https://review.couchbase.org/c/kv_engine/+/169677 Reviewed-by: Dave Rigby <daver@couchbase.com> Tested-by: Build Bot <build@couchbase.com>
couchbase · Jan 31, 2022 · 807db1c · 807db1c
1 parent 3b2626f
commit 807db1c
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 52 deletions.
diff --git a/engines/ep/src/kvstore/magma-kvstore/magma-kvstore.cc b/engines/ep/src/kvstore/magma-kvstore/magma-kvstore.cc
@@ -2415,8 +2415,7 @@ bool MagmaKVStore::compactDBInternal(std::unique_lock<std::mutex>& vbLock,
         try {
             ctx->completionCallback(*ctx);
         } catch (const std::exception& e) {
-            logger->critical("CompactionContext::completionCallback {}",
-                             e.what());
+            logger->error("CompactionContext::completionCallback {}", e.what());
             return false;
         }
     }

diff --git a/engines/ep/tests/module_tests/couchstore_bucket_tests.cc b/engines/ep/tests/module_tests/couchstore_bucket_tests.cc
@@ -947,56 +947,6 @@ TEST_P(STParamCouchstoreBucketTest, CompactionUpdatesBloomFilter) {
     EXPECT_EQ(expected, vb->getFilterSize());
 }
 
-TEST_P(STParamCouchstoreBucketTest,
-       RollbackCompletionCallbackStateAfterCompletionCallbackFailure) {
-    replaceCouchKVStoreWithMock();
-
-    setVBucketStateAndRunPersistTask(
-            vbid,
-            vbucket_state_active,
-            {{"topology", nlohmann::json::array({{"active", "replica"}})}});
-
-    auto vb = store->getVBucket(vbid);
-    auto newKey = makeStoredDocKey("key");
-    auto item = makePendingItem(newKey, "value");
-    EXPECT_EQ(cb::engine_errc::sync_write_pending, store->set(*item, cookie));
-    flushVBucketToDiskIfPersistent(vbid, 1);
-
-    EXPECT_EQ(cb::engine_errc::success,
-              vb->seqnoAcknowledged(
-                      folly::SharedMutex::ReadHolder(vb->getStateLock()),
-                      "replica",
-                      1));
-
-    vb->processResolvedSyncWrites();
-    flushVBucketToDiskIfPersistent(vbid, 1);
-
-    size_t collectionSize = 0;
-    {
-        Collections::Summary summary;
-        vb->getManifest().lock().updateSummary(summary);
-        EXPECT_LT(0, summary[CollectionID::Default].diskSize);
-        collectionSize = summary[CollectionID::Default].diskSize;
-    }
-
-    auto& mockEPBucket = dynamic_cast<MockEPBucket&>(*store);
-    mockEPBucket.setPostCompactionCompletionHook(
-            []() { throw std::runtime_error("oops"); });
-
-    runCompaction(vbid);
-
-    // Stats shouldn't change as we should abort the compaction
-    EXPECT_EQ(0, vb->getPurgeSeqno());
-    EXPECT_EQ(1, vb->getNumTotalItems());
-
-    {
-        Collections::Summary summary;
-        vb->getManifest().lock().updateSummary(summary);
-        EXPECT_EQ(1, summary[CollectionID::Default].itemCount);
-        EXPECT_EQ(collectionSize, summary[CollectionID::Default].diskSize);
-    }
-}
-
 /**
  * MB-42224: The test verifies that a failure in the header-sync phase at
  * flush-vbucket causes couchstore auto-retry. Also, the test verifies that

diff --git a/engines/ep/tests/module_tests/evp_store_single_threaded_test.cc b/engines/ep/tests/module_tests/evp_store_single_threaded_test.cc
@@ -5618,3 +5618,57 @@ TEST_P(STParamPersistentBucketTest,
     EXPECT_EQ(0, vb->dirtyQueueSize);
     EXPECT_FALSE(vb->isBucketCreation());
 }
+
+TEST_P(STParamPersistentBucketTest,
+       RollbackCompletionCallbackStateAfterCompletionCallbackFailure) {
+    if (isNexus()) {
+        // This test CAN run under Nexus but the callback is only forwarded on
+        // to the primary so it reports a mismatch as the primary compaction
+        // fails and the secondary does not.
+        GTEST_SKIP();
+    }
+    setVBucketStateAndRunPersistTask(
+            vbid,
+            vbucket_state_active,
+            {{"topology", nlohmann::json::array({{"active", "replica"}})}});
+
+    auto vb = store->getVBucket(vbid);
+    auto newKey = makeStoredDocKey("key");
+    auto item = makePendingItem(newKey, "value");
+    EXPECT_EQ(cb::engine_errc::sync_write_pending, store->set(*item, cookie));
+    flushVBucketToDiskIfPersistent(vbid, 1);
+
+    EXPECT_EQ(cb::engine_errc::success,
+              vb->seqnoAcknowledged(
+                      folly::SharedMutex::ReadHolder(vb->getStateLock()),
+                      "replica",
+                      1));
+
+    vb->processResolvedSyncWrites();
+    flushVBucketToDiskIfPersistent(vbid, 1);
+
+    size_t collectionSize = 0;
+    {
+        Collections::Summary summary;
+        vb->getManifest().lock().updateSummary(summary);
+        EXPECT_LT(0, summary[CollectionID::Default].diskSize);
+        collectionSize = summary[CollectionID::Default].diskSize;
+    }
+
+    auto& mockEPBucket = dynamic_cast<MockEPBucket&>(*store);
+    mockEPBucket.setPostCompactionCompletionHook(
+            []() { throw std::runtime_error("oops"); });
+
+    runCompaction(vbid);
+
+    // Stats shouldn't change as we should abort the compaction
+    EXPECT_EQ(0, vb->getPurgeSeqno());
+    EXPECT_EQ(1, vb->getNumTotalItems());
+
+    {
+        Collections::Summary summary;
+        vb->getManifest().lock().updateSummary(summary);
+        EXPECT_EQ(1, summary[CollectionID::Default].itemCount);
+        EXPECT_EQ(collectionSize, summary[CollectionID::Default].diskSize);
+    }
+}