From ac8385e4e17bdf867fb1aa7607f11aadf201acd1 Mon Sep 17 00:00:00 2001
From: Jim Walker <jim@couchbase.com>
Date: Wed, 21 Aug 2019 11:48:17 +0100
Subject: [PATCH] MB-35546: Return CAS from durable delete

Use the same technique from the set case where the
engine specific token is the CAS of the item.

Change-Id: I558b4b9071f5564ac9959dccf71ecc87c04bd0c0
Reviewed-on: http://review.couchbase.org/113632
Reviewed-by: Trond Norbye <trond.norbye@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
---
 engines/ep/src/ep_engine.cc               | 28 +++++++++++++----------
 engines/ep/src/ep_engine.h                |  4 ----
 tests/testapp_cluster/durability_tests.cc |  3 +--
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/engines/ep/src/ep_engine.cc b/engines/ep/src/ep_engine.cc
index f2c4eb01e3..0d73d99d8c 100644
--- a/engines/ep/src/ep_engine.cc
+++ b/engines/ep/src/ep_engine.cc
@@ -223,7 +223,7 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::remove(
         const boost::optional<cb::durability::Requirements>& durability,
         mutation_descr_t& mut_info) {
     return acquireEngine(this)->itemDelete(
-            cookie, key, cas, vbucket, durability, nullptr, mut_info);
+            cookie, key, cas, vbucket, durability, mut_info);
 }
 
 void EventuallyPersistentEngine::release(gsl::not_null<item*> itm) {
@@ -2140,23 +2140,27 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::itemDelete(
         uint64_t& cas,
         Vbid vbucket,
         boost::optional<cb::durability::Requirements> durability,
-        ItemMetaData* item_meta,
         mutation_descr_t& mut_info) {
     // Check if this is a in-progress durable delete which has now completed -
     // (see 'case EWOULDBLOCK' at the end of this function where we record
     // the fact we must block the client until the SycnWrite is durable).
-    if (durability && getEngineSpecific(cookie) != nullptr) {
-        // Non-null means this is the second call to this function after
-        // the SyncWrite has completed.
-        // Clear the engineSpecific, and return SUCCESS.
-        storeEngineSpecific(cookie, nullptr);
-        // @todo-durability - add support for non-sucesss (e.g. Aborted) when
-        // we support non-successful completions of SyncWrites.
-        return ENGINE_SUCCESS;
+    if (durability) {
+        void* deletedCas = getEngineSpecific(cookie);
+        if (deletedCas) {
+            // Non-null means this is the second call to this function after
+            // the SyncWrite has completed.
+            // Clear the engineSpecific, and return SUCCESS.
+            storeEngineSpecific(cookie, nullptr);
+
+            cas = reinterpret_cast<uint64_t>(deletedCas);
+            // @todo-durability - add support for non-sucesss (e.g. Aborted)
+            // when we support non-successful completions of SyncWrites.
+            return ENGINE_SUCCESS;
+        }
     }
 
     ENGINE_ERROR_CODE ret = kvBucket->deleteItem(
-            key, cas, vbucket, cookie, durability, item_meta, mut_info);
+            key, cas, vbucket, cookie, durability, nullptr, mut_info);
 
     switch (ret) {
     case ENGINE_KEY_ENOENT:
@@ -2174,7 +2178,7 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::itemDelete(
             // the result of the SyncWrite (see call to getEngineSpecific at
             // the head of this function).
             // (just store non-null value to indicate this).
-            storeEngineSpecific(cookie, reinterpret_cast<void*>(0x1));
+            storeEngineSpecific(cookie, reinterpret_cast<void*>(cas));
         }
         break;
 
diff --git a/engines/ep/src/ep_engine.h b/engines/ep/src/ep_engine.h
index 4c1a2c9982..4e9c4a9c44 100644
--- a/engines/ep/src/ep_engine.h
+++ b/engines/ep/src/ep_engine.h
@@ -409,9 +409,6 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
      *            back to the client
      * @param vbucket vbucket id to which the deleted key corresponds to
      * @param durability Optional durability requirements for this deletion.
-     * @param item_meta pointer to item meta data that needs to be
-     *                  as a result the delete. A NULL pointer indicates
-     *                  that no meta data needs to be returned.
      * @param mut_info pointer to the mutation info that resulted from
      *                 the delete.
      *
@@ -424,7 +421,6 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
             uint64_t& cas,
             Vbid vbucket,
             boost::optional<cb::durability::Requirements> durability,
-            ItemMetaData* item_meta,
             mutation_descr_t& mut_info);
 
     void itemRelease(item* itm);
diff --git a/tests/testapp_cluster/durability_tests.cc b/tests/testapp_cluster/durability_tests.cc
index 6f0d8ff2e6..70ca8dec55 100644
--- a/tests/testapp_cluster/durability_tests.cc
+++ b/tests/testapp_cluster/durability_tests.cc
@@ -108,8 +108,7 @@ TEST_F(DurabilityTest, Prepend) {
     mutate(*getConnection(), "Prepend", MutationType::Prepend);
 }
 
-// This is blocked by MB-35546
-TEST_F(DurabilityTest, DISABLED_Delete) {
+TEST_F(DurabilityTest, Delete) {
     auto conn = getConnection();
     const auto old =
             conn->store("Delete", Vbid{0}, "", cb::mcbp::Datatype::Raw);