Merge branch 'watson' 81/77881/2
authorDave Rigby <daver@couchbase.com>
Tue, 9 May 2017 10:43:12 +0000 (11:43 +0100)
committerDave Rigby <daver@couchbase.com>
Tue, 9 May 2017 10:45:44 +0000 (11:45 +0100)
* watson:
  MB-24066/MB-22178: Set opencheckpointid to 1 after rollback
  MB-24066: Partial revert "MB-22178: Don't use opencheckpointid to determine if in backfill phase"

Change-Id: If7d42bd933abf24a737d2718a4056e4536df2fd8

src/dcp/producer.cc
src/dcp/stream.cc
src/vbucket.cc
tests/module_tests/evp_store_rollback_test.cc

index 84e279b..7159dd3 100644 (file)
@@ -242,7 +242,7 @@ ENGINE_ERROR_CODE DcpProducer::streamRequest(uint32_t flags,
             logHeader(), vbucket, vb->toString(vb->getState()));
         return ENGINE_NOT_MY_VBUCKET;
     }
-    if (vb->isBackfillPhase()) {
+    if (vb->checkpointManager.getOpenCheckpointId() == 0) {
         LOG(EXTENSION_LOG_WARNING, "%s (vb %d) Stream request failed because "
             "this vbucket is in backfill state", logHeader(), vbucket);
         return ENGINE_TMPFAIL;
index 43bd23c..c7156bc 100644 (file)
@@ -2103,16 +2103,8 @@ void PassiveStream::processMarker(SnapshotMarker* marker) {
             vb->checkpointManager.setBackfillPhase(cur_snapshot_start.load(),
                                                    cur_snapshot_end.load());
         } else {
-            if (marker->getFlags() & MARKER_FLAG_CHK || vb->isBackfillPhase()) {
-                    // Instead of checking isBackfillPhase() an earlier patch
-                    // http://review.couchbase.org/#/c/39960/ changed the check
-                    // to (vb->checkpointManager.getOpenCheckpointId() == 0).
-                    // This change has been reverted in favor of using the
-                    // isBackfillPhase() check as it is believed to be safer and
-                    // makes the logic easier to understand.  However if an
-                    // issues such as that seen in MB-11786 occurs then it maybe
-                    // worth investigating the impact of changing the check to
-                    // use (vb->checkpointManager.getOpenCheckpointId() == 0)
+            if (marker->getFlags() & MARKER_FLAG_CHK ||
+                    vb->checkpointManager.getOpenCheckpointId() == 0) {
                 vb->checkpointManager.createSnapshot(cur_snapshot_start.load(),
                                                      cur_snapshot_end.load());
             } else {
index 9500426..309a54b 100644 (file)
@@ -1820,7 +1820,7 @@ void VBucket::postProcessRollback(const RollbackResult& rollbackResult,
     setPersistedSnapshot(rollbackResult.snapStartSeqno,
                          rollbackResult.snapEndSeqno);
     incrRollbackItemCount(prevHighSeqno - rollbackResult.highSeqno);
-    setBackfillPhase(false);
+    checkpointManager.setOpenCheckpointId(1);
 }
 
 void VBucket::dump() const {
index 6d207bb..16d3433 100644 (file)
@@ -288,10 +288,7 @@ TEST_P(RollbackTest, RollbackToMiddleOfAnUnPersistedSnapshot) {
 #endif
 
 /*
- * The opencheckpointid of a bucket can be zero after a rollback.
- * From MB21784 if an opencheckpointid was zero it was assumed that the
- * vbucket was in backfilling state.  This caused the producer stream
- * request to be stuck waiting for backfilling to complete.
+ * The opencheckpointid of a bucket is one after a rollback.
  */
 TEST_P(RollbackTest, MB21784) {
     // Make the vbucket a replica
@@ -301,10 +298,10 @@ TEST_P(RollbackTest, MB21784) {
         << "rollback did not return ENGINE_SUCCESS";
 
     // Assert the checkpointmanager clear function (called during rollback)
-    // has set the opencheckpointid to zero
+    // has set the opencheckpointid to one
     auto vb = store->getVBucket(vbid);
     auto& ckpt_mgr = vb->checkpointManager;
-    EXPECT_EQ(0, ckpt_mgr.getOpenCheckpointId()) << "opencheckpointId not zero";
+    EXPECT_EQ(1, ckpt_mgr.getOpenCheckpointId()) << "opencheckpointId not one";
 
     // Create a new Dcp producer, reserving its cookie.
     get_mock_server_api()->cookie->reserve(cookie);