MB-23713: increment the rev seqno while updating a deleted item

When setting the value for a deleted item, the rev seqno also
needs to be updated

Change-Id: I563e7ac896acbb9319db5fc3516d98ebc28d4d33
Reviewed-on: http://review.couchbase.org/76315
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>

diff --git a/src/vbucket.cc b/src/vbucket.cc
index 4a24c74..27942e9 100644 (file)
--- a/src/vbucket.cc
+++ b/src/vbucket.cc
@@ -1080,6 +1080,7 @@ ENGINE_ERROR_CODE VBucket::deleteItem(const DocKey& key,
          * delete.
          */
         if (v) {
+            itm->setRevSeqno(v->getRevSeqno());
             v->setValue(*itm, ht);
         } else {
             AddStatus rv = addTempStoredValue(hbl, key);

diff --git a/tests/ep_testsuite_basic.cc b/tests/ep_testsuite_basic.cc
index 5ba01d9..258acc8 100644 (file)
--- a/tests/ep_testsuite_basic.cc
+++ b/tests/ep_testsuite_basic.cc
@@ -1340,6 +1340,12 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
 
     h1->release(h, nullptr, i);
 
+    check(get_meta(h, h1, "key1"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    uint64_t curr_revseqno = last_meta.revSeqno;
+
     /* Store a deleted item first with CAS 0 */
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key1", "deletevalue", &i,
@@ -1348,6 +1354,13 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
 
     h1->release(h, nullptr, i);
 
+    check(get_meta(h, h1, "key1"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    checkeq(last_meta.revSeqno, curr_revseqno + 1,
+            "rev seqno should have incremented");
+
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key2", "somevalue", &i),
             "Failed set");
@@ -1358,6 +1371,12 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
 
     h1->release(h, nullptr, i);
 
+    check(get_meta(h, h1, "key2"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    curr_revseqno = last_meta.revSeqno;
+
     /* Store a deleted item with the existing CAS value */
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key2", "deletevaluewithcas",
@@ -1368,6 +1387,15 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
 
     wait_for_flusher_to_settle(h, h1);
 
+    check(get_meta(h, h1, "key2"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    checkeq(last_meta.revSeqno, curr_revseqno + 1,
+            "rev seqno should have incremented");
+
+    curr_revseqno = last_meta.revSeqno;
+
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key2",
                   "newdeletevalue", &i, 0, 0, 3600, 0x00, DocumentState::Deleted),
@@ -1379,6 +1407,15 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
 
     h1->release(h, nullptr, i);
 
+    check(get_meta(h, h1, "key2"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    checkeq(last_meta.revSeqno, curr_revseqno + 1,
+            "rev seqno should have incremented");
+
+    curr_revseqno = last_meta.revSeqno;
+
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key2",
                   "newdeletevaluewithcas", &i, info.cas, 0, 3600, 0x00,
@@ -1393,6 +1430,13 @@ static enum test_result test_delete_with_value(ENGINE_HANDLE *h,
                 DocStateFilter::AliveOrDeleted),
                 "Failed to get value");
 
+    check(get_meta(h, h1, "key2"), "Get meta failed");
+    checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+            last_status.load(), "Get meta failed");
+
+    checkeq(last_meta.revSeqno, curr_revseqno + 1,
+            "rev seqno should have incremented");
+
     check(h1->get_item_info(h, nullptr, i, &info),
           "Getting item info failed");
 

diff --git a/tests/module_tests/vbucket_test.cc b/tests/module_tests/vbucket_test.cc
index 60d3d5e..80b3294 100644 (file)
--- a/tests/module_tests/vbucket_test.cc
+++ b/tests/module_tests/vbucket_test.cc
@@ -326,13 +326,16 @@ TEST_P(VBucketTest, unlockedSoftDeleteWithValue) {
     Item deleted_item(key, 0, 0, "deletedvalue", strlen("deletedvalue"));
     deleted_item.setDeleted();
 
+    auto prev_revseqno = v->getRevSeqno();
+    deleted_item.setRevSeqno(prev_revseqno);
+
     // Set a new deleted value
     v->setValue(deleted_item, this->vbucket->ht);
 
-    ItemMetaData itm_meta;
     EXPECT_EQ(MutationStatus::WasDirty,
               this->public_processSoftDelete(v->getKey(), v, 0));
     verifyValue(key, "deletedvalue", TrackReference::Yes, WantsDeleted::Yes);
+    EXPECT_EQ(prev_revseqno + 1, v->getRevSeqno());
 }
 
 /**
@@ -363,6 +366,9 @@ TEST_P(VBucketTest, updateDeletedItem) {
     Item deleted_item(key, 0, 0, "deletedvalue", strlen("deletedvalue"));
     deleted_item.setDeleted();
 
+    auto prev_revseqno = v->getRevSeqno();
+    deleted_item.setRevSeqno(prev_revseqno);
+
     // Set a new deleted value
     v->setValue(deleted_item, this->vbucket->ht);
 
@@ -374,10 +380,15 @@ TEST_P(VBucketTest, updateDeletedItem) {
                 TrackReference::Yes,
                 WantsDeleted::Yes);
 
+    EXPECT_EQ(prev_revseqno + 1, v->getRevSeqno());
+
     Item update_deleted_item(
             key, 0, 0, "updatedeletedvalue", strlen("updatedeletedvalue"));
     update_deleted_item.setDeleted();
 
+    prev_revseqno = v->getRevSeqno();
+    update_deleted_item.setRevSeqno(prev_revseqno);
+
     // Set a new deleted value
     v->setValue(update_deleted_item, this->vbucket->ht);
 
@@ -385,6 +396,7 @@ TEST_P(VBucketTest, updateDeletedItem) {
               this->public_processSoftDelete(v->getKey(), v, 0));
     verifyValue(
             key, "updatedeletedvalue", TrackReference::Yes, WantsDeleted::Yes);
+    EXPECT_EQ(prev_revseqno + 1, v->getRevSeqno());
 }
 
 TEST_P(VBucketTest, SizeStatsSoftDel) {