MB-29816: Properly handle return codes from couchstore within couch-kvstore

Whilst this is not the cause of the issue, investigation into the code
paths led to the discovery we are not always handling the return codes
from couchstore within the saveDocs method. As such, fix this so we
are protected from potential failures.

Change-Id: If378efb1f89d2c8b3a169b5ac0265e8214a1dae3
Reviewed-on: http://review.couchbase.org/95371
Well-Formed: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: Tim Bradgate

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

@@ -1941,6 +1941,9 @@ static int readDocInfos(Db *db, DocInfo *docinfo, void *ctx) {
     if (ctx == nullptr) {
         throw std::invalid_argument("readDocInfos: ctx must be non-NULL");
     }
+    if (!docinfo) {
+        throw std::invalid_argument("readDocInfos: docInfo must be non-NULL");
+    }
     kvstats_ctx* cbCtx = static_cast<kvstats_ctx*>(ctx);
     if(docinfo) {
         // An item exists in the VB DB file.
@@ -1994,12 +1997,22 @@ couchstore_error_t CouchKVStore::saveDocs(uint16_t vbid,
                         ids[idx], configuration.shouldPersistDocNamespace());
                 kvctx.keyStats[key] = false;
             }
-            couchstore_docinfos_by_id(db,
-                                      ids.data(),
-                                      (unsigned)ids.size(),
-                                      0,
-                                      readDocInfos,
-                                      &kvctx);
+            errCode = couchstore_docinfos_by_id(db,
+                                                ids.data(),
+                                                (unsigned)ids.size(),
+                                                0,
+                                                readDocInfos,
+                                                &kvctx);
+            if (errCode != COUCHSTORE_SUCCESS) {
+                logger.log(EXTENSION_LOG_WARNING,
+                           "CouchKVStore::saveDocs: couchstore_docinfos_by_id "
+                           "error:%s [%s], vb:%" PRIu16 ", numdocs:%" PRIu64,
+                           couchstore_strerror(errCode),
+                           couchkvstore_strerrno(db, errCode).c_str(),
+                           vbid,
+                           uint64_t(docs.size()));
+                return errCode;
+            }
 
             auto cs_begin = ProcessClock::now();
             uint64_t flags = COMPRESS_DOC_BODIES | COUCHSTORE_SEQUENCE_AS_IS;
@@ -2053,7 +2066,15 @@ couchstore_error_t CouchKVStore::saveDocs(uint16_t vbid,
         st.batchSize.add(docs.size());
 
         // retrieve storage system stats for file fragmentation computation
-        couchstore_db_info(db, &info);
+        errCode = couchstore_db_info(db, &info);
+        if (errCode) {
+            logger.log(
+                    EXTENSION_LOG_WARNING,
+                    "CouchKVStore::saveDocs: couchstore_db_info error:%s [%s]",
+                    couchstore_strerror(errCode),
+                    couchkvstore_strerrno(db, errCode).c_str());
+            return errCode;
+        }
         cachedSpaceUsed[vbid] = info.space_used;
         cachedFileSize[vbid] = info.file_size;
         cachedDeleteCount[vbid] = info.deleted_count;

engines/ep/tests/module_tests/ep_unit_tests_main.cc

@@ -22,7 +22,7 @@
 #include "programs/engine_testapp/mock_server.h"
 
 #include <getopt.h>
-#include <gtest/gtest.h>
+#include <gmock/gmock.h>
 #include <logger/logger.h>
 
 #include "configuration.h"
@@ -67,6 +67,6 @@ int main(int argc, char **argv) {
     // Need to initialize ep_real_time and friends.
     initialize_time_functions(get_mock_server_api()->core);
 
-    ::testing::InitGoogleTest(&argc, argv);
+    ::testing::InitGoogleMock(&argc, argv);
     return RUN_ALL_TESTS();
 }

engines/ep/tests/module_tests/kvstore_test.cc

@@ -1079,6 +1079,48 @@ TEST_F(CouchKVStoreErrorInjectionTest, closeDB_close_file) {
     }
 }
 
+/**
+ * Injects error during CouchKVStore::saveDocs/couchstore_docinfos_by_id
+ */
+TEST_F(CouchKVStoreErrorInjectionTest, savedocs_doc_infos_by_id) {
+    // Insert some items into the B-Tree
+    generate_items(6);
+    CustomCallback<TransactionContext, mutation_result> set_callback;
+
+    for (const auto item : items) {
+        kvstore->begin(std::make_unique<TransactionContext>());
+        kvstore->set(item, set_callback);
+        kvstore->commit(nullptr /*no collections manifest*/);
+    }
+
+    {
+        generate_items(1);
+        CustomCallback<TransactionContext, mutation_result> set_callback;
+
+        kvstore->begin(std::make_unique<TransactionContext>());
+        kvstore->set(items.front(), set_callback);
+        {
+            /* Establish Logger expectation */
+            EXPECT_CALL(logger, mlog(_, _)).Times(AnyNumber());
+            EXPECT_CALL(
+                    logger,
+                    mlog(Ge(EXTENSION_LOG_WARNING), VCE(COUCHSTORE_ERROR_READ)))
+                    .Times(1)
+                    .RetiresOnSaturation();
+
+            /* Establish FileOps expectation */
+            EXPECT_CALL(ops, pread(_, _, _, _, _))
+                    .WillOnce(Return(COUCHSTORE_ERROR_READ))
+                    .RetiresOnSaturation();
+            EXPECT_CALL(ops, pread(_, _, _, _, _))
+                    .Times(4)
+                    .RetiresOnSaturation();
+
+            kvstore->commit(nullptr /*no collections manifest*/);
+        }
+    }
+}
+
 /**
  * Verify the failed compaction statistic is accurate.
  */