MB-41857: Only adjust file cache limit if open db successful

We should only adjust the file cache limit if the open db was successful, currently
we will decrement it for the file even if we cannot open it.

Change-Id: I8f8366012f4441073260f2e8065e48badf7240a2
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/137788
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: BenHuddleston

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

@@ -2017,10 +2017,9 @@ CouchKVStore::OpenResult CouchKVStore::openDb(Vbid vbucketId,
         return {COUCHSTORE_ERROR_OPEN_FILE, std::move(handle)};
     }
 
-    fc->decrementCacheSize();
-
     auto result = openSpecificDB(
             vbucketId, fileRev, handle->getDbHolder(), options, ops);
+
     if (result != COUCHSTORE_SUCCESS) {
         logger.debug(
                 "CouchKVStore::openDbForRead: {}  rev:{} - openSpecificDb "
@@ -2031,6 +2030,8 @@ CouchKVStore::OpenResult CouchKVStore::openDb(Vbid vbucketId,
         return {result, std::move(handle)};
     }
 
+    fc->decrementCacheSize();
+
     return {COUCHSTORE_SUCCESS, std::move(handle)};
 }
 

engines/ep/tests/module_tests/couch-kvstore_test.cc

@@ -622,6 +622,39 @@ TEST_F(CouchKVStoreErrorInjectionTest,
     }
 }
 
+// Test that if we fail to open the db we don't segfault by accessing a bad
+// ptr when we log the fileRev
+TEST_F(CouchKVStoreErrorInjectionTest,
+       FileCacheLimitIncreasedIfOpenForReadFails) {
+    vbid = Vbid(10);
+
+    // Make sure the vBucket does not exist before this test
+    ASSERT_FALSE(kvstore->getVBucketState(vbid));
+    ASSERT_THROW(kvstore->readVBState(vbid), std::logic_error);
+    ASSERT_EQ(0, kvstore->getKVStoreStat().numVbSetFailure);
+
+    auto cap = CouchKVStoreFileCache::get().getHandle()->capacity();
+    {
+        EXPECT_CALL(logger, mlog(_, _)).Times(AnyNumber());
+        EXPECT_CALL(logger, mlog(_, _)).Times(AnyNumber());
+        EXPECT_CALL(logger,
+                    mlog(Ge(spdlog::level::level_enum::warn),
+                         VCE(COUCHSTORE_ERROR_OPEN_FILE)))
+                .Times(1)
+                .RetiresOnSaturation();
+
+        /* Establish FileOps expectation */
+        EXPECT_CALL(ops, open(_, _, _, _))
+                .WillOnce(Return(COUCHSTORE_ERROR_OPEN_FILE))
+                .RetiresOnSaturation();
+
+        vbucket_state state;
+        kvstore->get(makeDiskDocKey("key"), vbid);
+    }
+
+    EXPECT_EQ(cap, CouchKVStoreFileCache::get().getHandle()->capacity());
+}
+
 /**
  * Injects error during CouchKVStore::openDB_retry/couchstore_open_db_ex
  */