MB-23999: Perform CAS check only if item being stored is alive 17/77217/6
authorSriram Ganesan <sriram@couchbase.com>
Fri, 21 Apr 2017 23:15:15 +0000 (16:15 -0700)
committerSriram Ganesan <sriram@couchbase.com>
Tue, 25 Apr 2017 14:14:02 +0000 (14:14 +0000)
If the existing document is expired, then storing another
deleted document with a CAS results in a ENOENT instead of
returning EEXISTS. The CAS check on an expired document is only
applicable if the incoming document is not in Deleted state

Change-Id: Ib6b78dd50236770a6be27a5fe341e321ef4eaec2
Reviewed-on: http://review.couchbase.org/77217
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
src/vbucket.cc
tests/module_tests/vbucket_test.cc

index dc3b6d9..d4aff69 100644 (file)
@@ -1934,7 +1934,7 @@ std::pair<MutationStatus, VBNotifyCtx> VBucket::processSet(
      * a cas operation for a key that doesn't exist is not a very cool
      * thing to do. See MB 3252
      */
-    if (v && v->isExpired(ep_real_time()) && !hasMetaData) {
+    if (v && v->isExpired(ep_real_time()) && !hasMetaData && !itm.isDeleted()) {
         if (v->isLocked(ep_current_time())) {
             v->unlock();
         }
index a23847d..43b7956 100644 (file)
@@ -340,6 +340,34 @@ TEST_P(VBucketTest, unlockedSoftDeleteWithValue) {
 }
 
 /**
+ * Test to check that if an item has expired, an incoming mutation
+ * on that item, if in deleted state results in an invalid cas and
+ * if not in deleted state, results in not found
+ */
+TEST_P(VBucketTest, updateExpiredItem) {
+    // Setup - create a key
+    StoredDocKey key = makeStoredDocKey("key");
+    Item stored_item(key, 0, ep_real_time() - 1, "value", strlen("value"));
+    ASSERT_EQ(MutationStatus::WasClean,
+              this->public_processSet(stored_item, stored_item.getCas()));
+
+    StoredValue* v = this->vbucket->ht.find(key, TrackReference::No, WantsDeleted::No);
+    EXPECT_TRUE(v);
+    EXPECT_TRUE(v->isExpired(ep_real_time()));
+
+    auto cas = v->getCas();
+    // Create an item and set its state to deleted.
+    Item deleted_item(key, 0, 0, "deletedvalue", strlen("deletedvalue"));
+
+    EXPECT_EQ(MutationStatus::NotFound,
+              this->public_processSet(deleted_item, cas + 1));
+
+    deleted_item.setDeleted();
+    EXPECT_EQ(MutationStatus::InvalidCas,
+              this->public_processSet(deleted_item, cas + 1));
+}
+
+/**
  * Test to check if an unlocked_softDelete performed on a
  * deleted item without a value and with a value
  */