From 56c9b8d8cc514de6c7a9342b8f47d5e06ead0d68 Mon Sep 17 00:00:00 2001 From: Scott Hernandez Date: Tue, 20 Sep 2016 01:22:56 -0400 Subject: SERVER-26179: Have CollectionBulkLoader::init use runner to execute work, not within runner task. --- src/mongo/db/repl/collection_bulk_loader.h | 4 +- src/mongo/db/repl/collection_bulk_loader_impl.cpp | 48 ++++++++++++----------- src/mongo/db/repl/collection_bulk_loader_impl.h | 4 +- src/mongo/db/repl/collection_cloner_test.cpp | 4 +- src/mongo/db/repl/data_replicator_test.cpp | 2 +- src/mongo/db/repl/database_cloner_test.cpp | 2 +- src/mongo/db/repl/databases_cloner_test.cpp | 2 +- src/mongo/db/repl/storage_interface_impl.cpp | 13 +++--- src/mongo/db/repl/storage_interface_mock.cpp | 5 +-- src/mongo/db/repl/storage_interface_mock.h | 11 ++---- 10 files changed, 44 insertions(+), 51 deletions(-) (limited to 'src/mongo/db/repl') 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& indexSpecs) = 0; + virtual Status init(Collection* coll, const std::vector& 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& 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::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 autoColl); virtual ~CollectionBulkLoaderImpl(); - virtual Status init(OperationContext* txn, - Collection* coll, - const std::vector& secondaryIndexSpecs) override; + virtual Status init(Collection* coll, const std::vector& secondaryIndexSpecs) override; virtual Status insertDocuments(const std::vector::const_iterator begin, const std::vector::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& secondaryIndexSpecs) { (_loader = new CollectionBulkLoaderMock(&collectionStats)) - ->init(nullptr, nullptr, secondaryIndexSpecs); + ->init(nullptr, secondaryIndexSpecs); return StatusWith>( std::unique_ptr(_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(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(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& secondaryIndexSpecs) { const auto collInfo = &_collections[nss]; (collInfo->loader = new CollectionBulkLoaderMock(&collInfo->stats)) - ->init(nullptr, nullptr, secondaryIndexSpecs); + ->init(nullptr, secondaryIndexSpecs); return StatusWith>( std::unique_ptr(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(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 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(txn, nss.db(), MODE_IX); auto coll = stdx::make_unique(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& secondaryIndexSpecs) { LOG(1) << "CollectionBulkLoaderMock::init called"; stats->initCalled = true; - return initFn(txn, coll, secondaryIndexSpecs); + return initFn(coll, secondaryIndexSpecs); }; Status CollectionBulkLoaderMock::insertDocuments(const std::vector::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& secondaryIndexSpecs) override; + virtual Status init(Collection* coll, const std::vector& secondaryIndexSpecs) override; virtual Status insertDocuments(const std::vector::const_iterator begin, const std::vector::const_iterator end) override; @@ -77,11 +75,8 @@ public: const std::vector::const_iterator) { return Status::OK(); }; stdx::function abortFn = []() { return Status::OK(); }; stdx::function commitFn = []() { return Status::OK(); }; - stdx::function& secondaryIndexSpecs)> - initFn = [](OperationContext*, Collection*, const std::vector&) { - return Status::OK(); - }; + stdx::function& secondaryIndexSpecs)> + initFn = [](Collection*, const std::vector&) { return Status::OK(); }; }; class StorageInterfaceMock : public StorageInterface { -- cgit v1.2.1