Skip to content

Commit

Permalink
MB-35546: Return CAS from durable delete
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>
Tested-by: Build Bot <[email protected]>
  • Loading branch information
jimwwalker authored and daverigby committed Aug 22, 2019
1 parent f21b690 commit ac8385e
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 18 deletions.
28 changes: 16 additions & 12 deletions engines/ep/src/ep_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
Expand All @@ -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;

Expand Down
4 changes: 0 additions & 4 deletions engines/ep/src/ep_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions tests/testapp_cluster/durability_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ac8385e

Please sign in to comment.