Permalink
Browse files

MB-6806 ep engine returns key not found for a deleted doc

This commit fixes the invalid error return (doc_not_found)
when the requested document is being marked deleted or when
the requested document is being compacted. In both cases,
ep engine now continues its mutation operation with the
default file revision number and let the callback handle the
error case with the key not found return instead.

Change-Id: I29c93eda060f16489830dc5d58f150ac1b0ec33a
Reviewed-on: http://review.couchbase.org/22023
Reviewed-by: Chiyoung Seo <chiyoung.seo@gmail.com>
Tested-by: Chiyoung Seo <chiyoung.seo@gmail.com>
  • Loading branch information...
1 parent 4fb2886 commit 386c78bc4a98001e4e09540b846bda93600e7207 @jinlim jinlim committed with steveyen Oct 26, 2012
Showing with 55 additions and 12 deletions.
  1. +12 −12 src/couch-kvstore/couch-kvstore.cc
  2. +43 −0 tests/ep_testsuite.cc
@@ -117,6 +117,8 @@ static int getMutationStatus(couchstore_error_t errCode)
switch (errCode) {
case COUCHSTORE_SUCCESS:
return MUTATION_SUCCESS;
+ case COUCHSTORE_ERROR_NO_HEADER:
+ case COUCHSTORE_ERROR_NO_SUCH_FILE:
case COUCHSTORE_ERROR_DOC_NOT_FOUND:
// this return causes ep engine to drop the failed flush
// of an item since it does not know about the itme any longer
@@ -491,23 +493,21 @@ void CouchKVStore::del(const Item &itm,
assert(!isReadOnly());
assert(intransaction);
std::string dbFile;
+ CouchRequestCallback requestcb;
+ uint64_t fileRev = 1;
+ requestcb.delCb = &cb;
if (getDbFile(itm.getVBucketId(), dbFile)) {
- CouchRequestCallback requestcb;
- requestcb.delCb = &cb;
- // each req will be de-allocated after commit
- CouchRequest *req = new CouchRequest(itm, dbFileRev(dbFile),
- requestcb, true);
- queueItem(req);
- } else {
- ++st.numDelFailure;
+ fileRev = dbFileRev(dbFile);
+ } else {
getLogger()->log(EXTENSION_LOG_WARNING, NULL,
- "Warning: failed to delete data, cannot locate "
- "database file %s\n",
+ "Warning: cannot locate database file %s for delete\n",
dbFile.c_str());
- int value = DOC_NOT_FOUND;
- cb.callback(value);
}
+
+ // each req will be de-allocated after commit
+ CouchRequest *req = new CouchRequest(itm, fileRev, requestcb, true);
+ queueItem(req);
}
bool CouchKVStore::delVBucket(uint16_t vbucket, bool recreate)
View
@@ -1298,6 +1298,47 @@ static enum test_result test_delete_set(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1)
return SUCCESS;
}
+static enum test_result test_get_delete_missing_file(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1) {
+ const char *key = "key";
+ wait_for_persisted_value(h, h1, key, "value2delete");
+
+ // whack the db file and directory where the key is stored
+ rmdb();
+
+ item *i = NULL;
+ int errorCode = h1->get(h, NULL, &i, key, strlen(key), 0);
+ h1->release(h, NULL, i);
+
+ // ep engine must be unaware of well-being of the db file as long as
+ // the item is still in the memory
+ check(errorCode == ENGINE_SUCCESS, "Expected success for get");
+
+ i = NULL;
+ evict_key(h, h1, key);
+ errorCode = h1->get(h, NULL, &i, key, strlen(key), 0);
+ h1->release(h, NULL, i);
+
+ // ep engine must be now aware of the ill-fated db file where
+ // the item is supposedly stored
+ check(errorCode == ENGINE_TMPFAIL, "Expected tmp fail for get");
+
+ int total_del_items = get_int_stat(h, h1, "ep_total_del_items");
+ int total_persisted = get_int_stat(h, h1, "ep_total_persisted");
+
+ // ep engine must still attempt to delete the item and return
+ // ENGINE_SUCCESS
+ errorCode = del(h, h1, "key", 0, 0);
+ check(errorCode == ENGINE_SUCCESS, "Expected success for del");
+
+ wait_for_flusher_to_settle(h, h1);
+ check(total_del_items == get_int_stat(h, h1, "ep_total_del_items"),
+ "expected no change in total_del_items stat");
+ check(total_persisted == get_int_stat(h, h1, "ep_total_persisted"),
+ "expected no change in total_persisted stat");
+ return SUCCESS;
+}
+
+
static enum test_result test_restart(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1) {
item *i = NULL;
static const char val[] = "somevalue";
@@ -6354,6 +6395,8 @@ engine_test_t* get_tests(void) {
test_setup, teardown, NULL, prepare, cleanup),
TestCase("delete/set/delete", test_delete_set, test_setup,
teardown, NULL, prepare, cleanup),
+ TestCase("get/delete with missing db file", test_get_delete_missing_file,
+ test_setup, teardown, NULL, prepare, cleanup),
TestCase("retain rowid over a soft delete", test_bug2509,
test_setup, teardown, NULL, prepare, cleanup),
TestCase("vbucket deletion doesn't affect new data", test_bug7023,

0 comments on commit 386c78b

Please sign in to comment.