SERVER-39703 Pull opCtx ptr out of the TemporaryKVRecordStore and instead pass it as a function parameter

7 files changed, 37 insertions, 5 deletions

diff --git a/src/mongo/db/catalog/database_test.cpp b/src/mongo/db/catalog/database_test.cpp
index 230e2f23251..9b44575217e 100644
--- a/src/mongo/db/catalog/database_test.cpp
+++ b/src/mongo/db/catalog/database_test.cpp
@@ -318,6 +318,7 @@ void _testDropCollectionThrowsExceptionIfThereAreIndexesInProgress(OperationCont
             WriteUnitOfWork wuow(opCtx);
             indexBuildBlock->success(opCtx, collection);
             wuow.commit();
+            indexBuildBlock->deleteTemporaryTables(opCtx);
         });
 
         ASSERT_GREATER_THAN(indexCatalog->numIndexesInProgress(opCtx), 0);
diff --git a/src/mongo/db/catalog/index_build_block.cpp b/src/mongo/db/catalog/index_build_block.cpp
index 3a3fb33eff1..1715fbf1c50 100644
--- a/src/mongo/db/catalog/index_build_block.cpp
+++ b/src/mongo/db/catalog/index_build_block.cpp
@@ -52,6 +52,12 @@ IndexCatalogImpl::IndexBuildBlock::IndexBuildBlock(IndexCatalogImpl* catalog,
                                                    IndexBuildMethod method)
     : _catalog(catalog), _ns(nss.ns()), _spec(spec.getOwned()), _method(method), _entry(nullptr) {}
 
+void IndexCatalogImpl::IndexBuildBlock::deleteTemporaryTables(OperationContext* opCtx) {
+    if (_indexBuildInterceptor) {
+        _indexBuildInterceptor->deleteTemporaryTables(opCtx);
+    }
+}
+
 Status IndexCatalogImpl::IndexBuildBlock::init(OperationContext* opCtx, Collection* collection) {
     // Being in a WUOW means all timestamping responsibility can be pushed up to the caller.
     invariant(opCtx->lockState()->inAWriteUnitOfWork());
diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h
index 59b4cb249cf..5ee4aed313c 100644
--- a/src/mongo/db/catalog/index_catalog.h
+++ b/src/mongo/db/catalog/index_catalog.h
@@ -156,6 +156,12 @@ public:
         virtual ~IndexBuildBlockInterface() = default;
 
         /**
+         * Must be called before the object is destructed if init() has been called.
+         * Cleans up the temporary tables that are created for an index build.
+         */
+        virtual void deleteTemporaryTables(OperationContext* opCtx) = 0;
+
+        /**
          * Initializes a new entry for the index in the IndexCatalog.
          *
          * On success, holds pointer to newly created IndexCatalogEntry that can be accessed using
diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h
index 943395c1df6..933bab58b62 100644
--- a/src/mongo/db/catalog/index_catalog_impl.h
+++ b/src/mongo/db/catalog/index_catalog_impl.h
@@ -252,6 +252,11 @@ public:
         ~IndexBuildBlock();
 
         /**
+         * Being called in a 'WriteUnitOfWork' has no effect.
+         */
+        void deleteTemporaryTables(OperationContext* opCtx);
+
+        /**
          * Must be called from within a `WriteUnitOfWork`
          */
         Status init(OperationContext* opCtx, Collection* collection);
diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index 552f0eafc49..55f8bf7bd80 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -95,6 +95,12 @@ void MultiIndexBlock::cleanUpAfterBuild(OperationContext* opCtx, Collection* col
     }
 
     if (!_needToCleanup || _indexes.empty()) {
+        // The temp tables cannot be dropped in commit() because commit() can be called multiple
+        // times on write conflict errors and the drop does not rollback in WUOWs.
+        for (auto& index : _indexes) {
+            index.block->deleteTemporaryTables(opCtx);
+        }
+
         _buildIsCleanedUp = true;
         return;
     }
@@ -109,6 +115,7 @@ void MultiIndexBlock::cleanUpAfterBuild(OperationContext* opCtx, Collection* col
             // a WUOW. Nothing inside this block can fail, and it is made fatal if it does.
             for (size_t i = 0; i < _indexes.size(); i++) {
                 _indexes[i].block->fail(opCtx, collection);
+                _indexes[i].block->deleteTemporaryTables(opCtx);
             }
 
             auto replCoord = repl::ReplicationCoordinator::get(opCtx);
@@ -228,7 +235,12 @@ StatusWith<std::vector<BSONObj>> MultiIndexBlock::init(OperationContext* opCtx,
     // On rollback in init(), cleans up _indexes so that ~MultiIndexBlock doesn't try to clean up
     // _indexes manually (since the changes were already rolled back).
     // Due to this, it is thus legal to call init() again after it fails.
-    opCtx->recoveryUnit()->onRollback([this]() { _indexes.clear(); });
+    opCtx->recoveryUnit()->onRollback([this, opCtx]() {
+        for (auto& index : _indexes) {
+            index.block->deleteTemporaryTables(opCtx);
+        }
+        _indexes.clear();
+    });
 
     const auto& ns = collection->ns().ns();
 
@@ -630,7 +642,6 @@ Status MultiIndexBlock::drainBackgroundWrites(OperationContext* opCtx,
     return Status::OK();
 }
 
-
 Status MultiIndexBlock::checkConstraints(OperationContext* opCtx) {
     if (State::kAborted == _getState()) {
         return {ErrorCodes::IndexBuildAborted,
@@ -657,8 +668,11 @@ Status MultiIndexBlock::checkConstraints(OperationContext* opCtx) {
     return Status::OK();
 }
 
-void MultiIndexBlock::abortWithoutCleanup() {
+void MultiIndexBlock::abortWithoutCleanup(OperationContext* opCtx) {
     _setStateToAbortedIfNotCommitted("aborted without cleanup"_sd);
+    for (auto& index : _indexes) {
+        index.block->deleteTemporaryTables(opCtx);
+    }
     _indexes.clear();
     _needToCleanup = false;
 }
diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h
index 98368293411..9ef56a6a998 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -260,7 +260,7 @@ public:
      *
      * Must be called from owning thread.
      */
-    void abortWithoutCleanup();
+    void abortWithoutCleanup(OperationContext* opCtx);
 
     /**
      * Returns true if this build block supports background writes while building an index. This is
diff --git a/src/mongo/db/catalog/multi_index_block_test.cpp b/src/mongo/db/catalog/multi_index_block_test.cpp
index e1e8c30133b..39adbc0728d 100644
--- a/src/mongo/db/catalog/multi_index_block_test.cpp
+++ b/src/mongo/db/catalog/multi_index_block_test.cpp
@@ -165,7 +165,7 @@ TEST_F(MultiIndexBlockTest, AbortWithoutCleanupAfterInsertingSingleDocument) {
         getOpCtx(), getCollection(), std::vector<BSONObj>(), MultiIndexBlock::kNoopOnInitFn));
     ASSERT_EQUALS(0U, specs.size());
     ASSERT_OK(indexer->insert(getOpCtx(), {}, {}));
-    indexer->abortWithoutCleanup();
+    indexer->abortWithoutCleanup(getOpCtx());
     ASSERT_EQUALS(MultiIndexBlock::State::kAborted, indexer->getState_forTest());
 
     ASSERT_FALSE(indexer->isCommitted());