SERVER-37763 MultiIndexBlock::commit() returns Status

This allows MultiIndexBlock to support aborting index builds.

23 files changed, 83 insertions, 44 deletions

diff --git a/src/mongo/db/catalog/collection_compact.cpp b/src/mongo/db/catalog/collection_compact.cpp
index 9eec0b1e8cb..96075e81a6c 100644
--- a/src/mongo/db/catalog/collection_compact.cpp
+++ b/src/mongo/db/catalog/collection_compact.cpp
@@ -150,7 +150,10 @@ StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
 
     {
         WriteUnitOfWork wunit(opCtx);
-        indexer.commit();
+        status = indexer.commit();
+        if (!status.isOK()) {
+            return StatusWith<CompactStats>(status);
+        }
         wunit.commit();
     }
 
diff --git a/src/mongo/db/catalog/database_test.cpp b/src/mongo/db/catalog/database_test.cpp
index 0fc1d321d38..b44c2c13c8f 100644
--- a/src/mongo/db/catalog/database_test.cpp
+++ b/src/mongo/db/catalog/database_test.cpp
@@ -303,7 +303,7 @@ void _testDropCollectionThrowsExceptionIfThereAreIndexesInProgress(OperationCont
         MultiIndexBlockImpl indexer(opCtx, collection);
         ON_BLOCK_EXIT([&indexer, opCtx] {
             WriteUnitOfWork wuow(opCtx);
-            indexer.commit();
+            ASSERT_OK(indexer.commit());
             wuow.commit();
         });
 
diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h
index cb526e2f3a9..4e72de1024d 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -165,8 +165,8 @@ public:
      *
      * Requires holding an exclusive database lock.
      */
-    virtual void commit() = 0;
-    virtual void commit(stdx::function<void(const BSONObj& spec)> onCreateFn) = 0;
+    virtual Status commit() = 0;
+    virtual Status commit(stdx::function<void(const BSONObj& spec)> onCreateFn) = 0;
 
     /**
      * May be called at any time after construction but before a successful commit(). Suppresses
diff --git a/src/mongo/db/catalog/multi_index_block_impl.cpp b/src/mongo/db/catalog/multi_index_block_impl.cpp
index f6394fa4af0..941b89e6c17 100644
--- a/src/mongo/db/catalog/multi_index_block_impl.cpp
+++ b/src/mongo/db/catalog/multi_index_block_impl.cpp
@@ -544,11 +544,11 @@ void MultiIndexBlockImpl::abortWithoutCleanup() {
     _needToCleanup = false;
 }
 
-void MultiIndexBlockImpl::commit() {
-    commit({});
+Status MultiIndexBlockImpl::commit() {
+    return commit({});
 }
 
-void MultiIndexBlockImpl::commit(stdx::function<void(const BSONObj& spec)> onCreateFn) {
+Status MultiIndexBlockImpl::commit(stdx::function<void(const BSONObj& spec)> onCreateFn) {
     // Do not interfere with writing multikey information when committing index builds.
     auto restartTracker =
         MakeGuard([this] { MultikeyPathTracker::get(_opCtx).startTrackingMultikeyPathInfo(); });
@@ -584,5 +584,8 @@ void MultiIndexBlockImpl::commit(stdx::function<void(const BSONObj& spec)> onCre
 
     _opCtx->recoveryUnit()->registerChange(new SetNeedToCleanupOnRollback(this));
     _needToCleanup = false;
+
+    return Status::OK();
 }
+
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/multi_index_block_impl.h b/src/mongo/db/catalog/multi_index_block_impl.h
index 7556dd058a3..b2a5c09110e 100644
--- a/src/mongo/db/catalog/multi_index_block_impl.h
+++ b/src/mongo/db/catalog/multi_index_block_impl.h
@@ -87,8 +87,8 @@ public:
     Status doneInserting(std::set<RecordId>* dupRecords) override;
     Status doneInserting(std::vector<BSONObj>* dupKeysInserted) override;
 
-    void commit() override;
-    void commit(stdx::function<void(const BSONObj& spec)> onCreateFn) override;
+    Status commit() override;
+    Status commit(stdx::function<void(const BSONObj& spec)> onCreateFn) override;
 
     void abortWithoutCleanup() override;
 
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp
index b3fe22e81d8..c3e71007b5e 100644
--- a/src/mongo/db/catalog/rename_collection.cpp
+++ b/src/mongo/db/catalog/rename_collection.cpp
@@ -467,14 +467,21 @@ Status renameCollectionCommon(OperationContext* opCtx,
             return status;
         }
 
-        writeConflictRetry(opCtx, "renameCollection", tmpName.ns(), [&] {
+        status = writeConflictRetry(opCtx, "renameCollection", tmpName.ns(), [&] {
             WriteUnitOfWork wunit(opCtx);
-            indexer.commit([opCtx, &tmpName, tmpColl](const BSONObj& spec) {
+            auto status = indexer.commit([opCtx, &tmpName, tmpColl](const BSONObj& spec) {
                 opCtx->getServiceContext()->getOpObserver()->onCreateIndex(
                     opCtx, tmpName, *(tmpColl->uuid()), spec, false);
             });
+            if (!status.isOK()) {
+                return status;
+            }
             wunit.commit();
+            return Status::OK();
         });
+        if (!status.isOK()) {
+            return status;
+        }
     }
 
     {
diff --git a/src/mongo/db/catalog/rename_collection_test.cpp b/src/mongo/db/catalog/rename_collection_test.cpp
index eebbc4a0819..3280f7836c2 100644
--- a/src/mongo/db/catalog/rename_collection_test.cpp
+++ b/src/mongo/db/catalog/rename_collection_test.cpp
@@ -420,7 +420,7 @@ void _createIndex(OperationContext* opCtx,
         MultiIndexBlockImpl indexer(opCtx, collection);
         ASSERT_OK(indexer.init(indexInfoObj).getStatus());
         WriteUnitOfWork wuow(opCtx);
-        indexer.commit();
+        ASSERT_OK(indexer.commit());
         wuow.commit();
     });
 
diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp
index fc62f8f59a5..cdc3703dcc4 100644
--- a/src/mongo/db/cloner.cpp
+++ b/src/mongo/db/cloner.cpp
@@ -405,7 +405,7 @@ void Cloner::copyIndexes(OperationContext* opCtx,
     uassertStatusOK(indexer.insertAllDocumentsInCollection());
 
     WriteUnitOfWork wunit(opCtx);
-    indexer.commit();
+    uassertStatusOK(indexer.commit());
     if (opCtx->writesAreReplicated()) {
         for (auto&& infoObj : indexInfoObjs) {
             getGlobalServiceContext()->getOpObserver()->onCreateIndex(
diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp
index 9b0cfae466d..15add881ec2 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -425,10 +425,10 @@ public:
         writeConflictRetry(opCtx, kCommandName, ns.ns(), [&] {
             WriteUnitOfWork wunit(opCtx);
 
-            indexer.commit([opCtx, &ns, collection](const BSONObj& spec) {
+            uassertStatusOK(indexer.commit([opCtx, &ns, collection](const BSONObj& spec) {
                 opCtx->getServiceContext()->getOpObserver()->onCreateIndex(
                     opCtx, ns, *(collection->uuid()), spec, false);
-            });
+            }));
 
             wunit.commit();
         });
diff --git a/src/mongo/db/commands/drop_indexes.cpp b/src/mongo/db/commands/drop_indexes.cpp
index b76d01f3b28..006e2b399ac 100644
--- a/src/mongo/db/commands/drop_indexes.cpp
+++ b/src/mongo/db/commands/drop_indexes.cpp
@@ -215,7 +215,7 @@ public:
 
         {
             WriteUnitOfWork wunit(opCtx);
-            indexer->commit();
+            uassertStatusOK(indexer->commit());
             wunit.commit();
         }
 
diff --git a/src/mongo/db/index/duplicate_key_tracker_test.cpp b/src/mongo/db/index/duplicate_key_tracker_test.cpp
index b7a49b0ab12..abb373e94e0 100644
--- a/src/mongo/db/index/duplicate_key_tracker_test.cpp
+++ b/src/mongo/db/index/duplicate_key_tracker_test.cpp
@@ -187,7 +187,7 @@ TEST_F(DuplicateKeyTrackerTest, IndexBuild) {
         AutoGetCollection tempColl(opCtx(), tracker->nss(), MODE_IX);
         ASSERT_OK(tracker->recordDuplicates(opCtx(), tempColl.getCollection(), dupsInserted));
 
-        indexer.commit();
+        ASSERT_OK(indexer.commit());
         wunit.commit();
     }
 
@@ -298,7 +298,7 @@ TEST_F(DuplicateKeyTrackerTest, BulkIndexBuild) {
         AutoGetCollection tempColl(opCtx(), tracker->nss(), MODE_IX);
         ASSERT_OK(tracker->recordDuplicates(opCtx(), tempColl.getCollection(), dupsInserted));
 
-        indexer.commit();
+        ASSERT_OK(indexer.commit());
         wunit.commit();
     }
 
diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index dde059c7254..72755ae13fc 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -246,9 +246,12 @@ Status IndexBuilder::_build(OperationContext* opCtx,
     if (allowBackgroundBuilding) {
         dbLock->relockWithMode(MODE_X);
     }
-    writeConflictRetry(opCtx, "Commit index build", ns.ns(), [opCtx, &indexer, &ns] {
+    status = writeConflictRetry(opCtx, "Commit index build", ns.ns(), [this, opCtx, &indexer, &ns] {
         WriteUnitOfWork wunit(opCtx);
-        indexer.commit();
+        auto status = indexer.commit();
+        if (!status.isOK()) {
+            return status;
+        }
 
         if (requiresGhostCommitTimestamp(opCtx, ns)) {
 
@@ -271,7 +274,11 @@ Status IndexBuilder::_build(OperationContext* opCtx,
             }
         }
         wunit.commit();
+        return Status::OK();
     });
+    if (!status.isOK()) {
+        return _failIndexBuild(indexer, status, allowBackgroundBuilding);
+    }
 
     if (allowBackgroundBuilding) {
         dbLock->relockWithMode(MODE_X);
diff --git a/src/mongo/db/index_rebuilder.cpp b/src/mongo/db/index_rebuilder.cpp
index e3cd2013df5..9478ad0a8c4 100644
--- a/src/mongo/db/index_rebuilder.cpp
+++ b/src/mongo/db/index_rebuilder.cpp
@@ -123,7 +123,7 @@ void checkNS(OperationContext* opCtx, const std::list<std::string>& nsToCheck) {
             uassertStatusOK(indexer.insertAllDocumentsInCollection());
 
             WriteUnitOfWork wunit(opCtx);
-            indexer.commit();
+            uassertStatusOK(indexer.commit());
             wunit.commit();
         } catch (const DBException& e) {
             error() << "Index rebuilding did not complete: " << redact(e);
diff --git a/src/mongo/db/repair_database.cpp b/src/mongo/db/repair_database.cpp
index 8958fdaa525..da970fb3a8b 100644
--- a/src/mongo/db/repair_database.cpp
+++ b/src/mongo/db/repair_database.cpp
@@ -219,7 +219,10 @@ Status rebuildIndexesOnCollection(OperationContext* opCtx,
 
     {
         WriteUnitOfWork wunit(opCtx);
-        indexer->commit();
+        status = indexer->commit();
+        if (!status.isOK()) {
+            return status;
+        }
         rs->updateStatsAfterRepair(opCtx, numRecords, dataSize);
         wunit.commit();
     }
diff --git a/src/mongo/db/repl/collection_bulk_loader_impl.cpp b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
index 4f1f7f5af23..29c1a7ea0e5 100644
--- a/src/mongo/db/repl/collection_bulk_loader_impl.cpp
+++ b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
@@ -175,11 +175,19 @@ Status CollectionBulkLoaderImpl::commit() {
                                             << " duplicates on secondary index(es) even though "
                                                "MultiIndexBlock::ignoreUniqueConstraint set."};
             }
-            writeConflictRetry(_opCtx.get(), "CollectionBulkLoaderImpl::commit", _nss.ns(), [this] {
-                WriteUnitOfWork wunit(_opCtx.get());
-                _secondaryIndexesBlock->commit();
-                wunit.commit();
-            });
+            status = writeConflictRetry(
+                _opCtx.get(), "CollectionBulkLoaderImpl::commit", _nss.ns(), [this] {
+                    WriteUnitOfWork wunit(_opCtx.get());
+                    auto status = _secondaryIndexesBlock->commit();
+                    if (!status.isOK()) {
+                        return status;
+                    }
+                    wunit.commit();
+                    return Status::OK();
+                });
+            if (!status.isOK()) {
+                return status;
+            }
         }
 
         if (_idIndexBlock) {
@@ -206,11 +214,19 @@ Status CollectionBulkLoaderImpl::commit() {
             }
 
             // Commit _id index, without dups.
-            writeConflictRetry(_opCtx.get(), "CollectionBulkLoaderImpl::commit", _nss.ns(), [this] {
-                WriteUnitOfWork wunit(_opCtx.get());
-                _idIndexBlock->commit();
-                wunit.commit();
-            });
+            status = writeConflictRetry(
+                _opCtx.get(), "CollectionBulkLoaderImpl::commit", _nss.ns(), [this] {
+                    WriteUnitOfWork wunit(_opCtx.get());
+                    auto status = _idIndexBlock->commit();
+                    if (!status.isOK()) {
+                        return status;
+                    }
+                    wunit.commit();
+                    return Status::OK();
+                });
+            if (!status.isOK()) {
+                return status;
+            }
         }
         _stats.endBuildingIndexes = Date_t::now();
         LOG(2) << "Done creating indexes for ns: " << _nss.ns() << ", stats: " << _stats.toString();
diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp
index ea8b0d1bbb6..146e453a77d 100644
--- a/src/mongo/db/repl/rs_rollback_test.cpp
+++ b/src/mongo/db/repl/rs_rollback_test.cpp
@@ -180,7 +180,7 @@ int createIndexForColl(OperationContext* opCtx,
     MultiIndexBlockImpl indexer(opCtx, coll);
     ASSERT_OK(indexer.init(indexSpec).getStatus());
     WriteUnitOfWork wunit(opCtx);
-    indexer.commit();
+    ASSERT_OK(indexer.commit());
     wunit.commit();
     auto indexCatalog = coll->getIndexCatalog();
     ASSERT(indexCatalog);
diff --git a/src/mongo/db/repl/storage_interface_impl_test.cpp b/src/mongo/db/repl/storage_interface_impl_test.cpp
index 0913f0a1c56..a02d85e0b1c 100644
--- a/src/mongo/db/repl/storage_interface_impl_test.cpp
+++ b/src/mongo/db/repl/storage_interface_impl_test.cpp
@@ -142,7 +142,7 @@ int createIndexForColl(OperationContext* opCtx, NamespaceString nss, BSONObj ind
     ASSERT_OK(indexer.init(indexSpec).getStatus());
 
     WriteUnitOfWork wunit(opCtx);
-    indexer.commit();
+    ASSERT_OK(indexer.commit());
     wunit.commit();
 
     auto indexCatalog = coll->getIndexCatalog();
diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp
index 4511d070a79..ec2f53aabac 100644
--- a/src/mongo/db/s/migration_destination_manager.cpp
+++ b/src/mongo/db/s/migration_destination_manager.cpp
@@ -660,7 +660,7 @@ void MigrationDestinationManager::cloneCollectionIndexesAndOptions(OperationCont
                     indexInfoObjs.isOK());
 
             WriteUnitOfWork wunit(opCtx);
-            indexer.commit();
+            uassertStatusOK(indexer.commit());
 
             for (auto&& infoObj : indexInfoObjs.getValue()) {
                 // make sure to create index on secondaries as well
diff --git a/src/mongo/db/system_index.cpp b/src/mongo/db/system_index.cpp
index 34ac8eca3a6..07f8b198d0e 100644
--- a/src/mongo/db/system_index.cpp
+++ b/src/mongo/db/system_index.cpp
@@ -136,10 +136,10 @@ void generateSystemIndexForExistingCollection(OperationContext* opCtx,
         writeConflictRetry(opCtx, "authorization index regeneration", ns.ns(), [&] {
             WriteUnitOfWork wunit(opCtx);
 
-            indexer.commit([opCtx, &ns, collection](const BSONObj& spec) {
+            fassert(51015, indexer.commit([opCtx, &ns, collection](const BSONObj& spec) {
                 opCtx->getServiceContext()->getOpObserver()->onCreateIndex(
                     opCtx, ns, *(collection->uuid()), spec, false /* fromMigrate */);
-            });
+            }));
 
             wunit.commit();
         });
diff --git a/src/mongo/dbtests/dbtests.cpp b/src/mongo/dbtests/dbtests.cpp
index 38a422bf936..e419bdf1015 100644
--- a/src/mongo/dbtests/dbtests.cpp
+++ b/src/mongo/dbtests/dbtests.cpp
@@ -118,7 +118,7 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj
         return status;
     }
     WriteUnitOfWork wunit(opCtx);
-    indexer.commit();
+    ASSERT_OK(indexer.commit());
     wunit.commit();
     return Status::OK();
 }
diff --git a/src/mongo/dbtests/indexupdatetests.cpp b/src/mongo/dbtests/indexupdatetests.cpp
index c3d5a62607b..9c44428ef6d 100644
--- a/src/mongo/dbtests/indexupdatetests.cpp
+++ b/src/mongo/dbtests/indexupdatetests.cpp
@@ -84,7 +84,7 @@ protected:
             uassertStatusOK(indexer.init(key));
             uassertStatusOK(indexer.insertAllDocumentsInCollection());
             WriteUnitOfWork wunit(&_opCtx);
-            indexer.commit();
+            ASSERT_OK(indexer.commit());
             wunit.commit();
         } catch (const DBException& e) {
             if (ErrorCodes::isInterruption(e.code()))
@@ -152,7 +152,7 @@ public:
         ASSERT_OK(indexer.insertAllDocumentsInCollection());
 
         WriteUnitOfWork wunit(&_opCtx);
-        indexer.commit();
+        ASSERT_OK(indexer.commit());
         wunit.commit();
     }
 };
@@ -385,7 +385,7 @@ Status IndexBuildBase::createIndex(const std::string& dbname, const BSONObj& ind
         return status;
     }
     WriteUnitOfWork wunit(&_opCtx);
-    indexer.commit();
+    ASSERT_OK(indexer.commit());
     wunit.commit();
     return Status::OK();
 }
diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp
index 7688df65a5c..a70c5570f8e 100644
--- a/src/mongo/dbtests/storage_timestamp_tests.cpp
+++ b/src/mongo/dbtests/storage_timestamp_tests.cpp
@@ -245,7 +245,7 @@ public:
         {
             WriteUnitOfWork wuow(_opCtx);
             // Timestamping index completion. Primaries write an oplog entry.
-            indexer.commit();
+            ASSERT_OK(indexer.commit());
             // The op observer is not called from the index builder, but rather the
             // `createIndexes` command.
             _opCtx->getServiceContext()->getOpObserver()->onCreateIndex(
@@ -1835,7 +1835,7 @@ public:
             // All callers of `MultiIndexBlock::commit` are responsible for timestamping index
             // completion.  Primaries write an oplog entry. Secondaries explicitly set a
             // timestamp.
-            indexer.commit();
+            ASSERT_OK(indexer.commit());
             if (SimulatePrimary) {
                 // The op observer is not called from the index builder, but rather the
                 // `createIndexes` command.
diff --git a/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp b/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
index 0c167e340f4..1f2e0e1334c 100644
--- a/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
+++ b/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
@@ -204,7 +204,7 @@ protected:
         ASSERT_OK(indexer.insertAllDocumentsInCollection());
 
         WriteUnitOfWork wunit(opCtx());
-        indexer.commit();
+        ASSERT_OK(indexer.commit());
         wunit.commit();
     }