MB-24294: Clean-up couch-kvstore openDB with reset=true

The openDB reset flag is only used by the CouchKVStore::reset method
when it calls set-state.

This is no longer required because of the changes in MB-23714 (which
also modified CouchKVStore::reset). There is no need to move the DB
file revision back to 1, it should be monotonically incrementing.

Change-Id: I50ba9873041157923ed99f34623f93ef67527641
Reviewed-on: http://review.couchbase.org/78007
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

diff --git a/src/couch-kvstore/couch-kvstore.cc b/src/couch-kvstore/couch-kvstore.cc
index a2629e5..f5ee558 100644 (file)
--- a/src/couch-kvstore/couch-kvstore.cc
+++ b/src/couch-kvstore/couch-kvstore.cc
@@ -396,8 +396,8 @@ void CouchKVStore::reset(uint16_t vbucketId) {
         unlinkCouchFile(vbucketId, dbFileRevMap[vbucketId]);
         incrementRevision(vbucketId);
 
-        setVBucketState(vbucketId, *state, VBStatePersist::VBSTATE_PERSIST_WITH_COMMIT,
-                        true);
+        setVBucketState(
+                vbucketId, *state, VBStatePersist::VBSTATE_PERSIST_WITH_COMMIT);
     } else {
         throw std::invalid_argument("CouchKVStore::reset: No entry in cached "
                         "states for vbucket " + std::to_string(vbucketId));
@@ -902,8 +902,10 @@ bool CouchKVStore::compactDB(compaction_ctx *hook_ctx) {
     hook_ctx->config = &configuration;
 
     // Open the source VBucket database file ...
-    errCode = openDB(vbid, fileRev, &compactdb,
-                     (uint64_t)COUCHSTORE_OPEN_FLAG_RDONLY, nullptr, false,
+    errCode = openDB(vbid,
+                     fileRev,
+                     &compactdb,
+                     (uint64_t)COUCHSTORE_OPEN_FLAG_RDONLY,
                      def_iops);
     if (errCode != COUCHSTORE_SUCCESS) {
         logger.log(EXTENSION_LOG_WARNING,
@@ -958,8 +960,8 @@ bool CouchKVStore::compactDB(compaction_ctx *hook_ctx) {
     }
 
     // Open the newly compacted VBucket database file ...
-    errCode = openDB(vbid, new_rev, &targetDb,
-                     (uint64_t)COUCHSTORE_OPEN_FLAG_RDONLY, NULL);
+    errCode = openDB(
+            vbid, new_rev, &targetDb, (uint64_t)COUCHSTORE_OPEN_FLAG_RDONLY);
     if (errCode != COUCHSTORE_SUCCESS) {
         logger.log(EXTENSION_LOG_WARNING,
                        "CouchKVStore::compactDB: openDB#2 error:%s, file:%s, "
@@ -1009,13 +1011,10 @@ vbucket_state * CouchKVStore::getVBucketState(uint16_t vbucketId) {
 }
 
 bool CouchKVStore::setVBucketState(uint16_t vbucketId,
-                                   const vbucket_state &vbstate,
-                                   VBStatePersist options,
-                                   bool reset) {
+                                   const vbucket_state& vbstate,
+                                   VBStatePersist options) {
     Db *db = NULL;
-    uint64_t fileRev, newFileRev;
-    std::stringstream id, rev;
-    std::string dbFileName;
+    uint64_t fileRev;
     std::map<uint16_t, uint64_t>::iterator mapItr;
     kvstats_ctx kvctx(configuration);
     kvctx.vbucket = vbucketId;
@@ -1023,33 +1022,29 @@ bool CouchKVStore::setVBucketState(uint16_t vbucketId,
 
     if (options == VBStatePersist::VBSTATE_PERSIST_WITHOUT_COMMIT ||
             options == VBStatePersist::VBSTATE_PERSIST_WITH_COMMIT) {
-        id << vbucketId;
         fileRev = dbFileRevMap[vbucketId];
-        rev << fileRev;
-        dbFileName = dbname + "/" + id.str() + ".couch." + rev.str();
-
-        errorCode = openDB(vbucketId, fileRev, &db,
-                           (uint64_t)COUCHSTORE_OPEN_FLAG_CREATE,
-                           &newFileRev, reset);
+        errorCode = openDB(
+                vbucketId, fileRev, &db, (uint64_t)COUCHSTORE_OPEN_FLAG_CREATE);
         if (errorCode != COUCHSTORE_SUCCESS) {
             ++st.numVbSetFailure;
             logger.log(EXTENSION_LOG_WARNING,
-                       "CouchKVStore::setVBucketState: openDB error:%s, name:%s",
-                       couchstore_strerror(errorCode), dbFileName.c_str());
+                       "CouchKVStore::setVBucketState: openDB error:%s, "
+                       "vb:%" PRIu16 ", fileRev:%" PRIu64,
+                       couchstore_strerror(errorCode),
+                       vbucketId,
+                       fileRev);
             return false;
         }
 
-        fileRev = newFileRev;
-        rev << fileRev;
-        dbFileName = dbname + "/" + id.str() + ".couch." + rev.str();
-
         errorCode = saveVBState(db, vbstate);
         if (errorCode != COUCHSTORE_SUCCESS) {
             ++st.numVbSetFailure;
             logger.log(EXTENSION_LOG_WARNING,
                        "CouchKVStore:setVBucketState: saveVBState error:%s, "
-                       "name:%s", couchstore_strerror(errorCode),
-                       dbFileName.c_str());
+                       "vb:%" PRIu16 ", fileRev:%" PRIu64,
+                       couchstore_strerror(errorCode),
+                       vbucketId,
+                       fileRev);
             closeDatabaseHandle(db);
             return false;
         }
@@ -1432,8 +1427,6 @@ couchstore_error_t CouchKVStore::openDB(uint16_t vbucketId,
                                         uint64_t fileRev,
                                         Db **db,
                                         uint64_t options,
-                                        uint64_t *newFileRev,
-                                        bool reset,
                                         FileOpsInterface* ops) {
     std::string dbFileName = getDBFileName(dbname, vbucketId, fileRev);
 
@@ -1454,60 +1447,40 @@ couchstore_error_t CouchKVStore::openDB(uint16_t vbucketId,
         options |= COUCHSTORE_OPEN_FLAG_UNBUFFERED;
     }
 
-    if (reset) {
-        errorCode = couchstore_open_db_ex(dbFileName.c_str(), options,
-                                          ops, db);
-        if (errorCode == COUCHSTORE_SUCCESS) {
-            newRevNum = 1;
-            updateDbFileMap(vbucketId, fileRev);
-            logger.log(EXTENSION_LOG_INFO,
-                       "CouchKVStore::openDB: created new couchstore file "
-                       "name:%s, rev:%" PRIu64 ", reset:true",
-                       dbFileName.c_str(), fileRev);
-        } else {
-            logger.log(EXTENSION_LOG_WARNING,
-                       "CouchKVStore::openDB: couchstore_open_db_ex error:%s, "
-                       "name:%s, rev:%" PRIu64 ", reset:true",
-                       couchstore_strerror(errorCode), dbFileName.c_str(),
-                       fileRev);
-        }
-    } else {
-        if (options & COUCHSTORE_OPEN_FLAG_CREATE) {
-            // first try to open the requested file without the
-            // create option in case it does already exist
-            errorCode = couchstore_open_db_ex(dbFileName.c_str(), 0, ops, db);
-            if (errorCode != COUCHSTORE_SUCCESS) {
-                // open_db failed but still check if the file exists
-                newRevNum = checkNewRevNum(dbFileName);
-                bool fileExists = (newRevNum) ? true : false;
-                if (fileExists) {
-                    errorCode = openDB_retry(dbFileName, 0, ops, db,
-                                             &newRevNum);
+    if (options & COUCHSTORE_OPEN_FLAG_CREATE) {
+        // first try to open the requested file without the
+        // create option in case it does already exist
+        errorCode = couchstore_open_db_ex(dbFileName.c_str(), 0, ops, db);
+        if (errorCode != COUCHSTORE_SUCCESS) {
+            // open_db failed but still check if the file exists
+            newRevNum = checkNewRevNum(dbFileName);
+            bool fileExists = (newRevNum) ? true : false;
+            if (fileExists) {
+                errorCode = openDB_retry(dbFileName, 0, ops, db, &newRevNum);
+            } else {
+                // requested file doesn't seem to exist, just create one
+                errorCode = couchstore_open_db_ex(
+                        dbFileName.c_str(), options, ops, db);
+                if (errorCode == COUCHSTORE_SUCCESS) {
+                    logger.log(EXTENSION_LOG_INFO,
+                               "CouchKVStore::openDB: created new couch db "
+                               "file, name:%s "
+                               "rev:%" PRIu64,
+                               dbFileName.c_str(),
+                               fileRev);
                 } else {
-                    // requested file doesn't seem to exist, just create one
-                    errorCode = couchstore_open_db_ex(dbFileName.c_str(),
-                                                      options, ops, db);
-                    if (errorCode == COUCHSTORE_SUCCESS) {
-                        newRevNum = 1;
-                        updateDbFileMap(vbucketId, fileRev);
-                        logger.log(EXTENSION_LOG_INFO,
-                                   "CouchKVStore::openDB: created new couch db file, name:%s "
-                                   "rev:%" PRIu64 ", reset:false",
-                                   dbFileName.c_str(), fileRev);
-                    } else {
-                        logger.log(EXTENSION_LOG_WARNING,
-                                   "CouchKVStore::openDB: couchstore_open_db_ex"
-                                   " error:%s, name:%s, rev:%" PRIu64
-                                   ", reset:false",
-                                   couchstore_strerror(errorCode),
-                                   dbFileName.c_str(), fileRev);
-                    }
+                    logger.log(EXTENSION_LOG_WARNING,
+                               "CouchKVStore::openDB: couchstore_open_db_ex"
+                               " error:%s, name:%s, rev:%" PRIu64
+                               ", reset:false",
+                               couchstore_strerror(errorCode),
+                               dbFileName.c_str(),
+                               fileRev);
                 }
             }
-        } else {
-            errorCode = openDB_retry(dbFileName, options, ops, db,
-                                     &newRevNum);
         }
+    } else {
+        errorCode = openDB_retry(dbFileName, options, ops, db, &newRevNum);
     }
 
     /* update command statistics */
@@ -1519,16 +1492,8 @@ couchstore_error_t CouchKVStore::openDB(uint16_t vbucketId,
                    couchstore_strerror(errorCode),
                    cb_strerror().c_str(), dbFileName.c_str(), options,
                    ((newRevNum > fileRev) ? newRevNum : fileRev));
-    } else {
-        if (newRevNum > fileRev) {
-            // new revision number found, update it
-            updateDbFileMap(vbucketId, newRevNum);
-        }
     }
 
-    if (newFileRev != NULL) {
-        *newFileRev = (newRevNum > fileRev) ? newRevNum : fileRev;
-    }
     return errorCode;
 }
 
@@ -1956,8 +1921,7 @@ couchstore_error_t CouchKVStore::saveDocs(uint16_t vbid,
     }
 
     DbHolder db(this);
-    uint64_t newFileRev;
-    errCode = openDB(vbid, fileRev, db.getDbAddress(), 0, &newFileRev);
+    errCode = openDB(vbid, fileRev, db.getDbAddress(), 0);
     if (errCode != COUCHSTORE_SUCCESS) {
         logger.log(EXTENSION_LOG_WARNING,
                    "CouchKVStore::saveDocs: openDB error:%s, vb:%" PRIu16

diff --git a/src/couch-kvstore/couch-kvstore.h b/src/couch-kvstore/couch-kvstore.h
index 9a850f6..0aad180 100644 (file)
--- a/src/couch-kvstore/couch-kvstore.h
+++ b/src/couch-kvstore/couch-kvstore.h
@@ -460,8 +460,9 @@ protected:
      */
     DbInfo getDbInfo(uint16_t vbid);
 
-    bool setVBucketState(uint16_t vbucketId, const vbucket_state &vbstate,
-                         VBStatePersist options, bool reset=false);
+    bool setVBucketState(uint16_t vbucketId,
+                         const vbucket_state& vbstate,
+                         VBStatePersist options);
 
     template <typename T>
     void addStat(const std::string &prefix, const char *nm, T &val,
@@ -477,9 +478,11 @@ protected:
                              std::vector<uint16_t> *vbids);
     void remVBucketFromDbFileMap(uint16_t vbucketId);
     void updateDbFileMap(uint16_t vbucketId, uint64_t newFileRev);
-    couchstore_error_t openDB(uint16_t vbucketId, uint64_t fileRev, Db **db,
-                              uint64_t options, uint64_t *newFileRev = NULL,
-                              bool reset=false, FileOpsInterface* ops = nullptr);
+    couchstore_error_t openDB(uint16_t vbucketId,
+                              uint64_t fileRev,
+                              Db** db,
+                              uint64_t options,
+                              FileOpsInterface* ops = nullptr);
     couchstore_error_t openDB_retry(std::string &dbfile, uint64_t options,
                                     FileOpsInterface *ops,
                                     Db **db, uint64_t *newFileRev);

diff --git a/tests/module_tests/kvstore_test.cc b/tests/module_tests/kvstore_test.cc
index 478f4a1..7123bad 100644 (file)
--- a/tests/module_tests/kvstore_test.cc
+++ b/tests/module_tests/kvstore_test.cc
@@ -158,6 +158,8 @@ protected:
 static void initialize_kv_store(KVStore* kvstore) {
     std::string failoverLog("");
     vbucket_state state(vbucket_state_active, 0, 0, 0, 0, 0, 0, 0, failoverLog);
+    // simulate the setVbState by incrementing the rev
+    kvstore->incrementRevision(0);
     kvstore->snapshotVBucket(0, state,
                             VBStatePersist::VBSTATE_PERSIST_WITHOUT_COMMIT);
 }
@@ -1164,6 +1166,8 @@ public:
         std::string failoverLog("");
         vbucket_state state(vbucket_state_active, 0, 0, 0, 0, 0, 0, 0,
                             failoverLog);
+        // simulate a setVBState - increment the dbFile revision
+        kvstore->incrementRevision(0);
         kvstore->snapshotVBucket(0, state,
                                  VBStatePersist::VBSTATE_PERSIST_WITHOUT_COMMIT);
     }