MB-23503: With an unpersisted snapshot, remove HT items till rollback point 28/75628/3
authorManu Dhundi <manu@couchbase.com>
Thu, 23 Mar 2017 18:52:06 +0000 (11:52 -0700)
committerManu Dhundi <manu@couchbase.com>
Thu, 23 Mar 2017 21:50:03 +0000 (21:50 +0000)
When the rollback request intends to have a rollback to a point in
an unpersisted snapshot, we must remove all unpersisted checkpoint
items from both checkpoint and hash table till the last persisted
disk snapshot.

Prior to this commit, we removed the items from the checkpoint
correctly, but only removed hash table items till the requested
rollback point, not all the unpersisted items, when
requested_rollback_seqno > persisted_seqno.

Note: We currently always rollback to a point which is persisted on
disk. Hence we must drop all checkpoint items from checkpoint and
hash table, irrespective of whether they are part of a full snapshot
or partial snapshot.

Change-Id: I1c14d0df6ae5e5459e60ecb0fc4a72ecd14231d3
Reviewed-on: http://review.couchbase.org/75628
Well-Formed: Build Bot <build@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
src/ep.cc
tests/module_tests/evp_store_rollback_test.cc

index fdcbe9f..3a776f5 100644 (file)
--- a/src/ep.cc
+++ b/src/ep.cc
@@ -3991,7 +3991,7 @@ EventuallyPersistentStore::rollback(uint16_t vbid,
             RollbackResult result = rwUnderlying->rollback(vbid, rollbackSeqno, cb);
 
             if (result.success) {
-                rollbackCheckpoint(vb, rollbackSeqno);
+                rollbackCheckpoint(vb, result.highSeqno);
                 vb->failovers->pruneEntries(result.highSeqno);
                 vb->checkpointManager.clear(vb, result.highSeqno);
                 vb->setPersistedSnapshot(result.snapStartSeqno, result.snapEndSeqno);
index ca8db9b..58ca61d 100644 (file)
@@ -224,14 +224,59 @@ TEST_P(RollbackTest, RollbackAfterDeletionNoFlush) {
     rollback_after_deletion_test(/*flush_before_rollback*/false);
 }
 
-TEST_P(RollbackTest, RollbackToMiddleOfACheckpoint) {
+TEST_P(RollbackTest, RollbackToMiddleOfAPersistedSnapshot) {
     rollback_to_middle_test(true);
 }
 
-TEST_P(RollbackTest, RollbackToMiddleOfACheckpointNoFlush) {
+TEST_P(RollbackTest, RollbackToMiddleOfAPersistedSnapshotNoFlush) {
     rollback_to_middle_test(false);
 }
 
+#if !defined(_MSC_VER) || _MSC_VER != 1800
+TEST_P(RollbackTest, RollbackToMiddleOfAnUnPersistedSnapshot) {
+    /* need to store a certain number of keys because rollback
+       'bails (rolls back to 0)' if the rollback is too much. */
+    const int numItems = 10;
+    for (int i = 0; i < numItems; i++) {
+        std::string key = "key_" + std::to_string(i);
+        store_item(vbid, key.c_str(), "not rolled back");
+    }
+
+    /* the roll back function will rewind disk to key11. */
+    auto rollback_item =
+    store_item(vbid, "key11", "rollback pt");
+
+    ASSERT_EQ(numItems + 1, store->flushVBucket(vbid));
+
+    /* Keys to be lost in rollback */
+    auto item_v1 = store_item(vbid, "rollback-cp-1", "hope to keep till here");
+    /* ask to rollback to here; this item is in a checkpoint and
+       is not persisted */
+    auto rollbackReqSeqno = item_v1.getBySeqno();
+
+    auto item_v2 = store_item(vbid, "rollback-cp-2", "gone");
+
+    /* do rollback */
+    store->setVBucketState(vbid, vbucket_state_replica, false);
+    EXPECT_EQ(ENGINE_SUCCESS, store->rollback(vbid, rollbackReqSeqno));
+
+    /* confirm that we have rolled back to the disk snapshot */
+    EXPECT_EQ(rollback_item.getBySeqno(),
+              store->getVBucket(vbid)->getHighSeqno());
+
+    /* since we rely only on disk snapshots currently, we must lose the items in
+       the checkpoints */
+    for (int i = 0; i < 2; i++) {
+        auto res = store->get(std::string("rollback-cp-" + std::to_string(i)),
+                              vbid,
+                              nullptr,
+                              {});
+        EXPECT_EQ(ENGINE_KEY_ENOENT, res.getStatus())
+                << "A key set after the rollback point was found";
+    }
+}
+#endif
+
 /*
  * The opencheckpointid of a bucket can be zero after a rollback.
  * From MB21784 if an opencheckpointid was zero it was assumed that the