MB-35408: Add KVStore::prepareToCreate

Add a KVStore method that intends to improve on the changes made in
32119d4. KVStore::prepareToCreate is called when a new VBucket is being
created and allows the KVStore and the backend implementation to perform
specific tasks for the creation.

prepareToCreate now does the following:

* Resets the KVStore vbucket cache for the new VB.

For couch-kvstore backend

* increments the revision number for the vbucket

All other backends currently do no further work.

The only snag to this clean-up is the way KV-engine structures the
read-write and read-only KVStores, we have a cache per store, but the
couch-kvstore file revision structure is shared between a rw/ro pair.

The change ensures that prepareToCreate is invoked on both rw/ro stores,
but the couch-kvstore implementation knows to only increment the
revision if the store is rw.

Change-Id: Ibc578157f122b680ae00c4048730d3eda61d1e9f
Reviewed-on: http://review.couchbase.org/112965
Reviewed-by: James Harrison <[email protected]>
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: jimwwalker

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

@@ -437,7 +437,7 @@ void CouchKVStore::reset(Vbid vbucketId) {
         // some higher level per VB lock is required to prevent data-races here.
         // KVBucket::vb_mutexes is used in this case.
         unlinkCouchFile(vbucketId, (*dbFileRevMap)[vbucketId.get()]);
-        incrementRevision(vbucketId);
+        prepareToCreateImpl(vbucketId);
 
         setVBucketState(
                 vbucketId, *state, VBStatePersist::VBSTATE_PERSIST_WITH_COMMIT);
@@ -3007,8 +3007,10 @@ void CouchKVStore::freeFileHandle(KVFileHandle* kvFileHandle) const {
     delete static_cast<CouchKVFileHandle*>(kvFileHandle);
 }
 
-void CouchKVStore::incrementRevision(Vbid vbid) {
-    (*dbFileRevMap)[vbid.get()]++;
+void CouchKVStore::prepareToCreateImpl(Vbid vbid) {
+    if (!isReadOnly()) {
+        (*dbFileRevMap)[vbid.get()]++;
+    }
 }
 
 uint64_t CouchKVStore::prepareToDeleteImpl(Vbid vbid) {

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

@@ -398,10 +398,11 @@ class CouchKVStore : public KVStore
     void freeFileHandle(KVFileHandle* kvFileHandle) const override;
 
     /**
-     * Increment the revision number of the vbucket.
+     * prepareToCreate will increment the revision number of the vbucket, but is
+     * a no-op if readOnly()
      * @param vbid ID of the vbucket to change.
      */
-    void incrementRevision(Vbid vbid) override;
+    void prepareToCreateImpl(Vbid vbid) override;
 
     /**
      * Prepare for delete of the vbucket file, this just removes the in-memory

engines/ep/src/kv_bucket.cc

@@ -982,12 +982,9 @@ ENGINE_ERROR_CODE KVBucket::setVBucketState_UNLOCKED(
         uint64_t start_chk_id = (to == vbucket_state_active) ? 2 : 0;
         newvb->checkpointManager->setOpenCheckpointId(start_chk_id);
 
-        // Before adding the VB to the map increment the revision
-        getRWUnderlying(vbid)->incrementRevision(vbid);
-
-        // Wipe out any cached vbucket state
-        getRWUnderlying(vbid)->resetCachedVBState(vbid);
-        getROUnderlying(vbid)->resetCachedVBState(vbid);
+        // Before adding the VB to the map, notify KVStore of the create
+        vbMap.getShardByVbId(vbid)->forEachKVStore(
+                [vbid](KVStore* kvs) { kvs->prepareToCreate(vbid); });
 
         // If active, update the VB from the bucket's collection state.
         // Note: Must be done /before/ adding the new VBucket to vbMap so that

engines/ep/src/kvshard.h

@@ -77,6 +77,14 @@ class KVShard {
         return rwStore.get();
     }
 
+    template <class UnaryFunction>
+    void forEachKVStore(UnaryFunction f) {
+        if (roStore) {
+            f(roStore.get());
+        }
+        f(rwStore.get());
+    }
+
     Flusher *getFlusher();
     BgFetcher *getBgFetcher();
 

engines/ep/src/kvstore.cc

@@ -497,6 +497,12 @@ uint64_t KVStore::prepareToDelete(Vbid vbid) {
     return prepareToDeleteImpl(vbid);
 }
 
+
+void KVStore::prepareToCreate(Vbid vbid) {
+    resetCachedVBState(vbid);
+    prepareToCreateImpl(vbid);
+}
+
 void KVStore::resetCachedVBState(Vbid vbid) {
     vbucket_state* state = getVBucketState(vbid);
     if (state) {

engines/ep/src/kvstore.h

@@ -897,12 +897,6 @@ class KVStore {
     virtual Collections::VB::PersistedStats getCollectionStats(
             const KVFileHandle& kvFileHandle, CollectionID collection) = 0;
 
-    /**
-     * Increment the revision number of the vbucket.
-     * @param vbid ID of the vbucket to change.
-     */
-    virtual void incrementRevision(Vbid vbid) = 0;
-
     /**
      * Prepare for delete of the vbucket file
      *
@@ -912,11 +906,10 @@ class KVStore {
     uint64_t prepareToDelete(Vbid vbid);
 
     /**
-     * Call reset on the cached vbucket_state for the given vbucket (if any is
-     * cached)
-     * @param vbid ID of the vbucket to reset
+     * Prepare for create of the vbucket
+     * @param vbid ID of the vbucket about to be created
      */
-    void resetCachedVBState(Vbid vbid);
+    void prepareToCreate(Vbid vbid);
 
     /**
      * Set a system event into the KVStore.
@@ -967,6 +960,14 @@ class KVStore {
      */
     virtual uint64_t prepareToDeleteImpl(Vbid vbid) = 0;
 
+    /*
+     * Prepare for a creation of the vbucket file - Implementation specific
+     * method that is called by prepareToCreate
+     *
+     * @param vbid ID of the vbucket being created
+     */
+    virtual void prepareToCreateImpl(Vbid vbid) = 0;
+
     /* all stats */
     KVStoreStats st;
     KVStoreConfig& configuration;
@@ -997,6 +998,13 @@ class KVStore {
      * @return true if the cached vbucket state is updated
      */
     bool updateCachedVBState(Vbid vbid, const vbucket_state& vbState);
+
+    /**
+     * Reset the cached state for a vbucket (see vbucket_state::reset)
+     *
+     * @param vbid the vbucket id to call reset on
+     */
+    void resetCachedVBState(Vbid vbid);
 };
 
 std::string to_string(KVStore::MutationStatus status);

engines/ep/src/magma-kvstore/magma-kvstore.h

@@ -243,7 +243,7 @@ class MagmaKVStore : public KVStore {
         return {};
     }
 
-    void incrementRevision(Vbid vbid) override {
+    void prepareToCreateImpl(Vbid vbid) override {
         // magma does not use file revisions
     }
 

engines/ep/src/rocksdb-kvstore/rocksdb-kvstore.h

@@ -311,7 +311,7 @@ class RocksDBKVStore : public KVStore {
         return {};
     }
 
-    void incrementRevision(Vbid vbid) override {
+    void prepareToCreateImpl(Vbid vbid) override {
         // TODO DJR 2017-05-19 implement this.
     }
 

engines/ep/tests/module_tests/kvstore_test.cc

@@ -187,11 +187,11 @@ class CustomRBCallback : public RollbackCB {
 // Initializes a KVStore
 static void initialize_kv_store(KVStore* kvstore, Vbid vbid = Vbid(0)) {
     // simulate the setVbState by incrementing the rev
-    kvstore->incrementRevision(vbid);
+    kvstore->prepareToCreate(vbid);
     vbucket_state state;
     state.state = vbucket_state_active;
     // simulate the setVbState by incrementing the rev
-    kvstore->incrementRevision(vbid);
+    kvstore->prepareToCreate(vbid);
     kvstore->snapshotVBucket(
             vbid, state, VBStatePersist::VBSTATE_PERSIST_WITHOUT_COMMIT);
 }
@@ -1428,11 +1428,11 @@ class CouchstoreTest : public ::testing::Test {
         std::string failoverLog("");
         // simulate a setVBState - increment the rev and then persist the
         // state
-        kvstore->incrementRevision(vbid);
+        kvstore->prepareToCreateImpl(vbid);
         vbucket_state state;
         state.state = vbucket_state_active;
         // simulate a setVBState - increment the dbFile revision
-        kvstore->incrementRevision(vbid);
+        kvstore->prepareToCreateImpl(vbid);
         kvstore->snapshotVBucket(
                 vbid, state, VBStatePersist::VBSTATE_PERSIST_WITHOUT_COMMIT);
     }