MB-41747: Reset the cached couch handle

With the introduction of a cache containing the vbucket files
we would reuse the couchstore handle every time we tried to
write to the database. That would work in the case where no
errors occur, but in the case for commit failures the data
was already written to the disk and the internal datastructures
in the couchstore handle referenced the new data (and it would
be included in the next commit).

On flush failures we would try to "roll back" the changes we
did and previously this worked due to the fact that we would
"drop" the couchstore instance every time we would try to add
data to the instance.

This patch resets the database handle every time it is fetched
from the cache. Ideally it should only be reset on failures
which would require it to do so, but that should be tracked in
its own MB.

Change-Id: I5f4809d9b5e266bb02a3c011e7316162201bab54
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/138740
Tested-by: Build Bot <[email protected]>
Reviewed-by: Jim Walker <[email protected]>

Author: trondn

engines/ep/src/couch-kvstore/couch-kvstore.cc

@@ -2125,6 +2125,16 @@ CouchKVStore::OpenForWriteResult CouchKVStore::openDbForWrite(Vbid vbucketId) {
                 closeOnUnlock};
     }
 
+    // MB-41747: couchstore_commit (and maybe other commands) may have returned
+    // an error and we don't really know the internal state. (in the case
+    // of a failing couchstore_commit the documents has been written to the
+    // database file and is linked from the internal data structures in the
+    // database handle and will be present in the _NEXT_ call to
+    // couchstore_commit. On the other hand our "flusher" tries to "roll back"
+    // these changes. This used to work before we added the cache, as each
+    // call would open the database file again. Until we've identified all
+    // cases lets reset the handle before returning the instance.
+    cb::couchstore::seek(*dbHolder->getDb(), cb::couchstore::Direction::End);
     return {COUCHSTORE_SUCCESS, std::move(dbHolder), closeOnUnlock};
 }
 

engines/ep/tests/module_tests/dcp_stream_test.cc

@@ -3838,6 +3838,19 @@ TEST_P(STPassiveStreamCouchstoreTest, VBStateNotLostAfterFlushFailure) {
     // engine
     replaceCouchKVStore(dynamic_cast<CouchKVStoreConfig&>(nonConstConfig),
                         *couchstore_get_default_file_ops());
+
+    // MB-41747: Make sure that we don't have a an on-disk-prepare as
+    // part of the internal database handle used by the underlying storage
+    // which will be written to disk (and purged as part of commit)
+    auto res = store->getLockedVBucket(vbid);
+    ASSERT_TRUE(res.owns_lock());
+    auto& underlying = *store->getRWUnderlying(vbid);
+
+    CompactionConfig cc;
+    cc.vbid = vbid;
+    auto context = std::make_shared<CompactionContext>(cc, 1);
+    underlying.compactDB(res.getLock(), context);
+    EXPECT_EQ(0, underlying.getVBucketState(vbid)->onDiskPrepares);
 }
 
 /**