MB-39292: 2/n Order set_collections with warm-up

Delay the acceptance of a new manifest from ns_server
whilst warm-up has yet to progress far enough.

This ensures that set_collections follows the loading
of any previously given manifest and KV can assert
that the new manifest is a valid successor.

Without this change KV can accept a manifest and then
proceed to load an older one from disk and fail within
warm-up as we cannot assign the older manifest over the
new one.

Change-Id: Ief2235b875001a77d9a184bb0b598e7fefbc1f5f
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/138166
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: jimwwalker

engines/ep/src/kv_bucket.cc

@@ -2628,6 +2628,16 @@ void KVBucket::initializeExpiryPager(Configuration& config) {
 
 cb::engine_error KVBucket::setCollections(std::string_view manifest,
                                           const void* cookie) {
+    // Only allow a new manifest once warmup has progressed past vbucket warmup
+    // 1) This means any prior manifest has been loaded
+    // 2) All vbuckets can have the new manifest applied
+    if (cookie && maybeWaitForVBucketWarmup(cookie)) {
+        EP_LOG_INFO("KVBucket::setCollections blocking for warmup cookie:{}",
+                    cookie);
+        return cb::engine_error(cb::engine_errc::would_block,
+                                "KVBucket::setCollections waiting for warmup");
+    }
+
     // Inhibit VB state changes whilst updating the vbuckets
     LockHolder lh(vbsetMutex);
 

engines/ep/tests/module_tests/collections/evp_store_collections_test.cc

@@ -1254,11 +1254,19 @@ TEST_F(CollectionsWarmupTest, MB_38125) {
     resetEngineAndEnableWarmup();
 
     CollectionsManifest cm(CollectionEntry::fruit);
-    setCollections(cookie, std::string{cm});
+
+    // Cannot set the manifest yet - command follows ewouldblock pattern
+    auto status = engine->set_collection_manifest(cookie, std::string{cm});
+    EXPECT_EQ(cb::engine_errc::would_block, status);
+    cookie_to_mock_cookie(cookie)->status = ENGINE_FAILED;
 
     // Now get the engine warmed up
     runReadersUntilWarmedUp();
 
+    // cookie now notified and setCollections can go ahead
+    EXPECT_EQ(ENGINE_SUCCESS, cookie_to_mock_cookie(cookie)->status);
+    setCollections(cookie, std::string{cm});
+
     auto vb = store->getVBucket(vbid);
 
     // Fruit is enabled
@@ -1275,7 +1283,11 @@ TEST_F(CollectionsWarmupTest, LockedVBStateDuringManifestUpdate) {
     // complete warmup
     CollectionsManifest cm;
     cm.remove(CollectionEntry::defaultC);
-    setCollections(cookie, std::string{cm});
+
+    // Cannot set the manifest yet - command follows ewouldblock pattern
+    auto status = engine->set_collection_manifest(cookie, std::string{cm});
+    EXPECT_EQ(cb::engine_errc::would_block, status);
+    cookie_to_mock_cookie(cookie)->status = ENGINE_FAILED;
 
     auto* mockEPBucket = dynamic_cast<MockEPBucket*>(engine->getKVBucket());
     mockEPBucket->setCollectionsManagerPreSetStateAtWarmupHook([this]() {
@@ -1288,6 +1300,10 @@ TEST_F(CollectionsWarmupTest, LockedVBStateDuringManifestUpdate) {
     });
 
     runReadersUntilWarmedUp();
+
+    // cookie now notified and setCollections can go ahead
+    EXPECT_EQ(ENGINE_SUCCESS, cookie_to_mock_cookie(cookie)->status);
+    setCollections(cookie, std::string{cm});
 }
 
 /**

engines/ep/tests/module_tests/evp_store_single_threaded_test.cc

@@ -389,7 +389,14 @@ cb::engine_errc SingleThreadedKVBucketTest::setCollections(
     }
 
     auto& lpAuxioQ = *task_executor->getLpTaskQ()[AUXIO_TASK_IDX];
+
+    cookie_to_mock_cookie(c)->status = ENGINE_FAILED;
+
     runNextTask(lpAuxioQ);
+
+    // Cookie now success
+    EXPECT_EQ(ENGINE_SUCCESS, cookie_to_mock_cookie(c)->status);
+
     status = engine->set_collection_manifest(c, json);
     EXPECT_EQ(cb::engine_errc::success, status);
     return status;