MB-41942: Merge branch '6.5.2' into mad-hatter

* 6.5.2:
  MB-41942: Persist the VB state earlier during warmup

Change-Id: Ia5967038242f9f7a9e9c25afca50f639c9a8d394

Author: BenHuddleston

engines/ep/src/ep_bucket.cc

@@ -1907,10 +1907,6 @@ bool EPBucket::maybeEnableTraffic() {
 }
 
 void EPBucket::warmupCompleted() {
-    // Snapshot VBucket state after warmup to ensure Failover table is
-    // persisted.
-    scheduleVBStatePersist();
-
     if (engine.getConfiguration().getAlogPath().length() > 0) {
         if (engine.getConfiguration().isAccessScannerEnabled()) {
             {

engines/ep/src/warmup.cc

@@ -1063,6 +1063,12 @@ void Warmup::createVBuckets(uint16_t shardId) {
                 } else {
                     vb->failovers->createEntry(vbs.lastSnapStart);
                 }
+
+                // MB-41942: Persist the latest VB state to ensure that the
+                // FailoverTable is correct if we write any new seqnos during
+                // warmup (e.g. due to expiration)
+                vb->checkpointManager->queueSetVBState(*vb);
+
                 auto entry = vb->failovers->getLatestEntry();
                 EP_LOG_INFO(
                         "Warmup::createVBuckets: {} created new failover entry "

engines/ep/tests/module_tests/evp_store_single_threaded_test.cc

@@ -52,6 +52,7 @@
 #include "tests/test_fileops.h"
 #include "vbucket_bgfetch_item.h"
 #include "vbucket_state.h"
+#include "warmup.h"
 
 #include "../couchstore/src/internal.h"
 
@@ -5038,6 +5039,126 @@ void STParamPersistentBucketTest::testAbortDoesNotIncrementOpsDelete(
     EXPECT_EQ(0, vb.opsDelete);
 }
 
+void STParamPersistentBucketTest::testFailoverTableEntryPersistedAtWarmup(
+        std::function<void()> testFunction) {
+    // 1) Store something so we can expire it later
+    engine->getKVBucket()->setVBucketState(vbid, vbucket_state_active);
+    auto vb = engine->getKVBucket()->getVBucket(vbid);
+
+    // Grab initialUuid for testing
+    auto initialUuid = vb->failovers->getLatestUUID();
+
+    auto key = makeStoredDocKey("key");
+    store_item(vbid, key, "value", 1 /*expiryTime*/);
+    flushVBucketToDiskIfPersistent(vbid, 1);
+    EXPECT_EQ(1, vb->getHighSeqno());
+
+    // 2) Restart as though we had an unclean shutdown (creating a new failover
+    //    table entry) and run the warmup up to the point of completion.
+    vb.reset();
+    resetEngineAndEnableWarmup();
+
+    auto& readerQueue = *task_executor->getLpTaskQ()[READER_TASK_IDX];
+    auto* warmup = engine->getKVBucket()->getWarmup();
+    ASSERT_TRUE(warmup);
+
+    // Warmup - load everything but don't run the complete phase which schedules
+    // persistence of the vBucket state (new failover entry)
+    while (warmup->getWarmupState() != WarmupState::State::Done) {
+        runNextTask(readerQueue);
+    }
+
+    // 3) Test
+    testFunction();
+
+    // New high seqno
+    vb = engine->getKVBucket()->getVBucket(vbid);
+    EXPECT_EQ(2, vb->getHighSeqno());
+
+    // Flush the expiry
+    flushVBucketToDiskIfPersistent(vbid, 1);
+
+    // Verify that the item has been expired
+    auto options = static_cast<get_options_t>(
+            QUEUE_BG_FETCH | HONOR_STATES | TRACK_REFERENCE | DELETE_TEMP |
+            HIDE_LOCKED_CAS | TRACK_STATISTICS);
+    auto gv = store->get(key, vbid, cookie, options);
+
+    if (gv.getStatus() == ENGINE_EWOULDBLOCK) {
+        runBGFetcherTask();
+        gv = store->get(key, vbid, cookie, options);
+    }
+
+    EXPECT_EQ(ENGINE_KEY_ENOENT, gv.getStatus());
+
+    // Get our new uuid now
+    auto secondUuid = vb->failovers->getLatestUUID();
+    ASSERT_NE(initialUuid, secondUuid);
+
+    // "Complete" the warmup or the test will get stuck shutting down, we won't
+    // actually flush the new vb state though so we're still testing as though
+    // this didn't happen
+    runNextTask(readerQueue);
+
+    // 4) Restart again
+    vb.reset();
+    resetEngineAndWarmup();
+
+    // 5) The test - uuid should have both of the previous entries
+    vb = engine->getKVBucket()->getVBucket(vbid);
+
+    auto failovers = vb->failovers->getFailoverLog();
+    auto itr = std::find_if(failovers.begin(),
+                            failovers.end(),
+                            [&initialUuid](const auto& failoverEntry) {
+                                return failoverEntry.uuid == initialUuid;
+                            });
+    EXPECT_NE(itr, failovers.end());
+
+    itr = std::find_if(failovers.begin(),
+                       failovers.end(),
+                       [&secondUuid](const auto& failoverEntry) {
+                           return failoverEntry.uuid == secondUuid;
+                       });
+    EXPECT_NE(itr, failovers.end());
+
+    EXPECT_EQ(2, vb->getHighSeqno());
+    gv = store->get(key, vbid, cookie, options);
+
+    if (gv.getStatus() == ENGINE_EWOULDBLOCK) {
+        runBGFetcherTask();
+        gv = store->get(key, vbid, cookie, options);
+    }
+}
+
+TEST_P(STParamPersistentBucketTest,
+       TestExpiryDueToCompactionPersistsFailoverTableEntryDuringWarmup) {
+    testFailoverTableEntryPersistedAtWarmup([this]() {
+        CompactionConfig config;
+        engine->compactDB(vbid, config, cookie);
+        std::string taskDescription =
+                "Compact DB file " + std::to_string(vbid.get());
+        runNextTask(*task_executor->getLpTaskQ()[WRITER_TASK_IDX],
+                    taskDescription);
+    });
+}
+
+TEST_P(STParamPersistentBucketTest,
+       TestExpiryDueToGetPersistsFailoverTableEntryDuringWarmup) {
+    testFailoverTableEntryPersistedAtWarmup([this]() {
+        auto options = static_cast<get_options_t>(
+                QUEUE_BG_FETCH | HONOR_STATES | TRACK_REFERENCE | DELETE_TEMP |
+                HIDE_LOCKED_CAS | TRACK_STATISTICS);
+        auto key = makeStoredDocKey("key");
+        auto gv = store->get(key, vbid, cookie, options);
+
+        if (gv.getStatus() == ENGINE_EWOULDBLOCK) {
+            runBGFetcherTask();
+            gv = store->get(key, vbid, cookie, options);
+        }
+    });
+}
+
 TEST_P(STParamPersistentBucketTest, AbortDoesNotIncrementOpsDelete) {
     testAbortDoesNotIncrementOpsDelete(true /*flusherDedup*/);
 }

engines/ep/tests/module_tests/evp_store_single_threaded_test.h

@@ -373,4 +373,6 @@ class STParamPersistentBucketTest : public STParameterizedBucketTest {
      * @param dropDeletes compaction config param
      */
     void testCompactionPersistedDeletes(bool dropDeletes);
+
+    void testFailoverTableEntryPersistedAtWarmup(std::function<void()>);
 };