SERVER-26179: Have CollectionBulkLoader::init use runner to execute work, not within runner task.

10 files changed, 44 insertions, 51 deletions

diff --git a/src/mongo/db/repl/collection_bulk_loader.h b/src/mongo/db/repl/collection_bulk_loader.h
index f3f5e0cb6e5..3884c16673c 100644
--- a/src/mongo/db/repl/collection_bulk_loader.h
+++ b/src/mongo/db/repl/collection_bulk_loader.h
@@ -45,9 +45,7 @@ class CollectionBulkLoader {
 public:
     virtual ~CollectionBulkLoader() = default;
 
-    virtual Status init(OperationContext* txn,
-                        Collection* coll,
-                        const std::vector<BSONObj>& indexSpecs) = 0;
+    virtual Status init(Collection* coll, const std::vector<BSONObj>& indexSpecs) = 0;
     /**
      * Inserts the documents into the collection record store, and indexes them with the
      * MultiIndexBlock on the side.
diff --git a/src/mongo/db/repl/collection_bulk_loader_impl.cpp b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
index 8bc7a20efdc..5fdb64218b9 100644
--- a/src/mongo/db/repl/collection_bulk_loader_impl.cpp
+++ b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
@@ -86,31 +86,33 @@ CollectionBulkLoaderImpl::~CollectionBulkLoaderImpl() {
     })
 }
 
-Status CollectionBulkLoaderImpl::init(OperationContext* txn,
-                                      Collection* coll,
+Status CollectionBulkLoaderImpl::init(Collection* coll,
                                       const std::vector<BSONObj>& secondaryIndexSpecs) {
-    invariant(txn);
-    invariant(coll);
-    invariant(txn->getClient() == &cc());
-    if (secondaryIndexSpecs.size()) {
-        _secondaryIndexesBlock->ignoreUniqueConstraint();
-        auto status = _secondaryIndexesBlock->init(secondaryIndexSpecs).getStatus();
-        if (!status.isOK()) {
-            return status;
-        }
-    } else {
-        _secondaryIndexesBlock.reset();
-    }
-    if (!_idIndexSpec.isEmpty()) {
-        auto status = _idIndexBlock->init(_idIndexSpec).getStatus();
-        if (!status.isOK()) {
-            return status;
-        }
-    } else {
-        _idIndexBlock.reset();
-    }
+    return _runTaskReleaseResourcesOnFailure(
+        [coll, &secondaryIndexSpecs, this](OperationContext* txn) -> Status {
+            invariant(txn);
+            invariant(coll);
+            invariant(txn->getClient() == &cc());
+            if (secondaryIndexSpecs.size()) {
+                _secondaryIndexesBlock->ignoreUniqueConstraint();
+                auto status = _secondaryIndexesBlock->init(secondaryIndexSpecs).getStatus();
+                if (!status.isOK()) {
+                    return status;
+                }
+            } else {
+                _secondaryIndexesBlock.reset();
+            }
+            if (!_idIndexSpec.isEmpty()) {
+                auto status = _idIndexBlock->init(_idIndexSpec).getStatus();
+                if (!status.isOK()) {
+                    return status;
+                }
+            } else {
+                _idIndexBlock.reset();
+            }
 
-    return Status::OK();
+            return Status::OK();
+        });
 }
 
 Status CollectionBulkLoaderImpl::insertDocuments(const std::vector<BSONObj>::const_iterator begin,
diff --git a/src/mongo/db/repl/collection_bulk_loader_impl.h b/src/mongo/db/repl/collection_bulk_loader_impl.h
index b66baeb2bae..61928e4c385 100644
--- a/src/mongo/db/repl/collection_bulk_loader_impl.h
+++ b/src/mongo/db/repl/collection_bulk_loader_impl.h
@@ -70,9 +70,7 @@ public:
                              std::unique_ptr<AutoGetCollection> autoColl);
     virtual ~CollectionBulkLoaderImpl();
 
-    virtual Status init(OperationContext* txn,
-                        Collection* coll,
-                        const std::vector<BSONObj>& secondaryIndexSpecs) override;
+    virtual Status init(Collection* coll, const std::vector<BSONObj>& secondaryIndexSpecs) override;
 
     virtual Status insertDocuments(const std::vector<BSONObj>::const_iterator begin,
                                    const std::vector<BSONObj>::const_iterator end) override;
diff --git a/src/mongo/db/repl/collection_cloner_test.cpp b/src/mongo/db/repl/collection_cloner_test.cpp
index a92ebf3e674..6c31428d2bd 100644
--- a/src/mongo/db/repl/collection_cloner_test.cpp
+++ b/src/mongo/db/repl/collection_cloner_test.cpp
@@ -89,7 +89,7 @@ void CollectionClonerTest::setUp() {
                const BSONObj idIndexSpec,
                const std::vector<BSONObj>& secondaryIndexSpecs) {
             (_loader = new CollectionBulkLoaderMock(&collectionStats))
-                ->init(nullptr, nullptr, secondaryIndexSpecs);
+                ->init(nullptr, secondaryIndexSpecs);
 
             return StatusWith<std::unique_ptr<CollectionBulkLoader>>(
                 std::unique_ptr<CollectionBulkLoader>(_loader));
@@ -209,7 +209,7 @@ TEST_F(CollectionClonerTest, DoNotCreateIDIndexIfAutoIndexIdUsed) {
             collNss = theNss;
             collOptions = theOptions;
             collIndexSpecs = theIndexSpecs;
-            loader->init(nullptr, nullptr, theIndexSpecs);
+            loader->init(nullptr, theIndexSpecs);
             return std::unique_ptr<CollectionBulkLoader>(loader);
         };
 
diff --git a/src/mongo/db/repl/data_replicator_test.cpp b/src/mongo/db/repl/data_replicator_test.cpp
index e51912b7878..2d23d46ed71 100644
--- a/src/mongo/db/repl/data_replicator_test.cpp
+++ b/src/mongo/db/repl/data_replicator_test.cpp
@@ -267,7 +267,7 @@ protected:
                     log() << "reusing collection during test which may cause problems, ns:" << nss;
                 }
                 (collInfo->loader = new CollectionBulkLoaderMock(&collInfo->stats))
-                    ->init(nullptr, nullptr, secondaryIndexSpecs);
+                    ->init(nullptr, secondaryIndexSpecs);
 
                 return StatusWith<std::unique_ptr<CollectionBulkLoader>>(
                     std::unique_ptr<CollectionBulkLoader>(collInfo->loader));
diff --git a/src/mongo/db/repl/database_cloner_test.cpp b/src/mongo/db/repl/database_cloner_test.cpp
index 73cda0fc674..94e93be5be4 100644
--- a/src/mongo/db/repl/database_cloner_test.cpp
+++ b/src/mongo/db/repl/database_cloner_test.cpp
@@ -98,7 +98,7 @@ void DatabaseClonerTest::setUp() {
                const std::vector<BSONObj>& secondaryIndexSpecs) {
             const auto collInfo = &_collections[nss];
             (collInfo->loader = new CollectionBulkLoaderMock(&collInfo->stats))
-                ->init(nullptr, nullptr, secondaryIndexSpecs);
+                ->init(nullptr, secondaryIndexSpecs);
 
             return StatusWith<std::unique_ptr<CollectionBulkLoader>>(
                 std::unique_ptr<CollectionBulkLoader>(collInfo->loader));
diff --git a/src/mongo/db/repl/databases_cloner_test.cpp b/src/mongo/db/repl/databases_cloner_test.cpp
index 071de224a60..110db06f9d8 100644
--- a/src/mongo/db/repl/databases_cloner_test.cpp
+++ b/src/mongo/db/repl/databases_cloner_test.cpp
@@ -176,7 +176,7 @@ protected:
                     log() << "reusing collection during test which may cause problems, ns:" << nss;
                 }
                 (collInfo->loader = new CollectionBulkLoaderMock(&collInfo->stats))
-                    ->init(nullptr, nullptr, secondaryIndexSpecs);
+                    ->init(nullptr, secondaryIndexSpecs);
 
                 return StatusWith<std::unique_ptr<CollectionBulkLoader>>(
                     std::unique_ptr<CollectionBulkLoader>(collInfo->loader));
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index 7a568856493..c36497609d0 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -251,6 +251,7 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
 
     // Setup cond_var for signalling when done.
     std::unique_ptr<CollectionBulkLoader> loaderToReturn;
+    Collection* collection;
 
     auto status = runner->runSynchronousTask([&](OperationContext* txn) -> Status {
         // We are not replicating nor validating writes under this OperationContext*.
@@ -264,7 +265,7 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
             ScopedTransaction transaction(txn, MODE_IX);
             auto db = stdx::make_unique<AutoGetOrCreateDb>(txn, nss.db(), MODE_IX);
             auto coll = stdx::make_unique<AutoGetCollection>(txn, nss, MODE_X);
-            Collection* collection = coll->getCollection();
+            collection = coll->getCollection();
 
             if (collection) {
                 return {ErrorCodes::NamespaceExists, "Collection already exists."};
@@ -285,11 +286,6 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
                                                                       std::move(runner),
                                                                       std::move(db),
                                                                       std::move(coll));
-            invariant(collection);
-            auto status = loader->init(txn, collection, secondaryIndexSpecs);
-            if (!status.isOK()) {
-                return status;
-            }
 
             // Move the loader into the StatusWith.
             loaderToReturn = std::move(loader);
@@ -303,6 +299,11 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
         return status;
     }
 
+    invariant(collection);
+    status = loaderToReturn->init(collection, secondaryIndexSpecs);
+    if (!status.isOK()) {
+        return status;
+    }
     return std::move(loaderToReturn);
 }
 
diff --git a/src/mongo/db/repl/storage_interface_mock.cpp b/src/mongo/db/repl/storage_interface_mock.cpp
index 7104824ebb0..ec870c1a0cc 100644
--- a/src/mongo/db/repl/storage_interface_mock.cpp
+++ b/src/mongo/db/repl/storage_interface_mock.cpp
@@ -91,12 +91,11 @@ OpTime StorageInterfaceMock::getAppliedThrough(OperationContext* txn) {
     return _appliedThrough;
 }
 
-Status CollectionBulkLoaderMock::init(OperationContext* txn,
-                                      Collection* coll,
+Status CollectionBulkLoaderMock::init(Collection* coll,
                                       const std::vector<BSONObj>& secondaryIndexSpecs) {
     LOG(1) << "CollectionBulkLoaderMock::init called";
     stats->initCalled = true;
-    return initFn(txn, coll, secondaryIndexSpecs);
+    return initFn(coll, secondaryIndexSpecs);
 };
 
 Status CollectionBulkLoaderMock::insertDocuments(const std::vector<BSONObj>::const_iterator begin,
diff --git a/src/mongo/db/repl/storage_interface_mock.h b/src/mongo/db/repl/storage_interface_mock.h
index 6eb5bf14556..a11cf789ef1 100644
--- a/src/mongo/db/repl/storage_interface_mock.h
+++ b/src/mongo/db/repl/storage_interface_mock.h
@@ -53,9 +53,7 @@ class CollectionBulkLoaderMock : public CollectionBulkLoader {
 public:
     CollectionBulkLoaderMock(CollectionMockStats* collStats) : stats(collStats){};
     virtual ~CollectionBulkLoaderMock() = default;
-    virtual Status init(OperationContext* txn,
-                        Collection* coll,
-                        const std::vector<BSONObj>& secondaryIndexSpecs) override;
+    virtual Status init(Collection* coll, const std::vector<BSONObj>& secondaryIndexSpecs) override;
 
     virtual Status insertDocuments(const std::vector<BSONObj>::const_iterator begin,
                                    const std::vector<BSONObj>::const_iterator end) override;
@@ -77,11 +75,8 @@ public:
                           const std::vector<BSONObj>::const_iterator) { return Status::OK(); };
     stdx::function<Status()> abortFn = []() { return Status::OK(); };
     stdx::function<Status()> commitFn = []() { return Status::OK(); };
-    stdx::function<Status(
-        OperationContext* txn, Collection* coll, const std::vector<BSONObj>& secondaryIndexSpecs)>
-        initFn = [](OperationContext*, Collection*, const std::vector<BSONObj>&) {
-            return Status::OK();
-        };
+    stdx::function<Status(Collection* coll, const std::vector<BSONObj>& secondaryIndexSpecs)>
+        initFn = [](Collection*, const std::vector<BSONObj>&) { return Status::OK(); };
 };
 
 class StorageInterfaceMock : public StorageInterface {