summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/mongo/base/error_codes.err1
-rw-r--r--src/mongo/db/catalog/SConscript2
-rw-r--r--src/mongo/db/catalog/multi_index_block.h28
-rw-r--r--src/mongo/db/catalog/multi_index_block_impl.cpp109
-rw-r--r--src/mongo/db/catalog/multi_index_block_impl.h52
-rw-r--r--src/mongo/db/catalog/multi_index_block_test.cpp204
6 files changed, 384 insertions, 12 deletions
diff --git a/src/mongo/base/error_codes.err b/src/mongo/base/error_codes.err
index 5dcfaf5d878..ca74eb7a13b 100644
--- a/src/mongo/base/error_codes.err
+++ b/src/mongo/base/error_codes.err
@@ -273,6 +273,7 @@ error_code("MigrationConflict", 272)
error_code("ProducerConsumerQueueProducerQueueDepthExceeded", 273)
error_code("ProducerConsumerQueueConsumed", 274)
error_code("ExchangePassthrough", 275) # For exchange execution in aggregation. Do not reuse.
+error_code("IndexBuildAborted", 276)
# Error codes 4000-8999 are reserved.
# Non-sequential error codes (for compatibility only)
diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript
index 7195387ad6c..06ef36e5bce 100644
--- a/src/mongo/db/catalog/SConscript
+++ b/src/mongo/db/catalog/SConscript
@@ -235,6 +235,8 @@ env.CppUnitTest(
'collection',
'multi_index_block',
'$BUILD_DIR/mongo/db/namespace_string',
+ '$BUILD_DIR/mongo/db/repl/replmocks',
+ '$BUILD_DIR/mongo/db/service_context_test_fixture',
],
)
diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h
index 4e72de1024d..70d2d4f0ce9 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -37,6 +37,7 @@
#include "mongo/base/disallow_copying.h"
#include "mongo/base/status.h"
+#include "mongo/base/string_data.h"
#include "mongo/db/catalog/index_catalog.h"
#include "mongo/db/index/index_access_method.h"
#include "mongo/db/record_id.h"
@@ -169,6 +170,31 @@ public:
virtual Status commit(stdx::function<void(const BSONObj& spec)> onCreateFn) = 0;
/**
+ * Returns true if this index builder was added to the index catalog successfully.
+ * In addition to having commit() return without errors, the enclosing WUOW has to be committed
+ * for the indexes to show up in the index catalog.
+ */
+ virtual bool isCommitted() const = 0;
+
+ /**
+ * Signals the index build to abort.
+ *
+ * In-progress inserts and commits will still run to completion. However, subsequent index build
+ * operations will fail an IndexBuildAborted error.
+ *
+ * Aborts the uncommitted index build and prevents further inserts or commit attempts from
+ * proceeding. On destruction, all traces of uncommitted index builds will be removed.
+ *
+ * If the index build has already been aborted (using abort() or abortWithoutCleanup()),
+ * this function does nothing.
+ *
+ * If this index build has been committed successfully, this function has no effect.
+ *
+ * May be called from any thread.
+ */
+ virtual void abort(StringData reason) = 0;
+
+ /**
* May be called at any time after construction but before a successful commit(). Suppresses
* the default behavior on destruction of removing all traces of uncommitted index builds.
*
@@ -181,6 +207,8 @@ public:
*
* Does not matter whether it is called inside of a WriteUnitOfWork. Will not be rolled
* back.
+ *
+ * Must be called from owning thread.
*/
virtual void abortWithoutCleanup() = 0;
diff --git a/src/mongo/db/catalog/multi_index_block_impl.cpp b/src/mongo/db/catalog/multi_index_block_impl.cpp
index 941b89e6c17..28f90ae1249 100644
--- a/src/mongo/db/catalog/multi_index_block_impl.cpp
+++ b/src/mongo/db/catalog/multi_index_block_impl.cpp
@@ -34,6 +34,8 @@
#include "mongo/db/catalog/multi_index_block_impl.h"
+#include <ostream>
+
#include "mongo/base/error_codes.h"
#include "mongo/base/init.h"
#include "mongo/db/audit.h"
@@ -50,6 +52,7 @@
#include "mongo/db/operation_context.h"
#include "mongo/db/repl/replication_coordinator.h"
#include "mongo/db/server_parameters.h"
+#include "mongo/logger/redaction.h"
#include "mongo/stdx/memory.h"
#include "mongo/stdx/mutex.h"
#include "mongo/util/fail_point.h"
@@ -209,6 +212,19 @@ StatusWith<std::vector<BSONObj>> MultiIndexBlockImpl::init(const BSONObj& spec)
}
StatusWith<std::vector<BSONObj>> MultiIndexBlockImpl::init(const std::vector<BSONObj>& indexSpecs) {
+ if (State::kAborted == _getState()) {
+ return {ErrorCodes::IndexBuildAborted,
+ str::stream() << "Index build aborted: " << _abortReason
+ << ". Cannot initialize index builder: "
+ << _collection->ns().ns()
+ << "("
+ << *_collection->uuid()
+ << "): "
+ << indexSpecs.size()
+ << " provided. First index spec: "
+ << (indexSpecs.empty() ? BSONObj() : indexSpecs[0])};
+ }
+
WriteUnitOfWork wunit(_opCtx);
invariant(_indexes.empty());
@@ -313,6 +329,8 @@ StatusWith<std::vector<BSONObj>> MultiIndexBlockImpl::init(const std::vector<BSO
}
}
+ _setState(State::kRunning);
+
return indexInfoObjs;
}
@@ -481,6 +499,17 @@ Status MultiIndexBlockImpl::insertAllDocumentsInCollection() {
Status MultiIndexBlockImpl::insert(const BSONObj& doc,
const RecordId& loc,
std::vector<BSONObj>* const dupKeysInserted) {
+ if (State::kAborted == _getState()) {
+ return {ErrorCodes::IndexBuildAborted,
+ str::stream() << "Index build aborted: " << _abortReason
+ << ". Cannot insert document into index builder: "
+ << _collection->ns().ns()
+ << "("
+ << *_collection->uuid()
+ << "): "
+ << redact(doc)};
+ }
+
for (size_t i = 0; i < _indexes.size(); i++) {
if (_indexes[i].filterExpression && !_indexes[i].filterExpression->matchesBSON(doc)) {
continue;
@@ -516,9 +545,18 @@ Status MultiIndexBlockImpl::doneInserting(std::set<RecordId>* dupRecords) {
Status MultiIndexBlockImpl::doneInserting(std::vector<BSONObj>* dupKeysInserted) {
return _doneInserting(nullptr, dupKeysInserted);
}
-
Status MultiIndexBlockImpl::_doneInserting(std::set<RecordId>* dupRecords,
std::vector<BSONObj>* dupKeysInserted) {
+ if (State::kAborted == _getState()) {
+ return {ErrorCodes::IndexBuildAborted,
+ str::stream() << "Index build aborted: " << _abortReason
+ << ". Cannot complete insertion phase: "
+ << _collection->ns().ns()
+ << "("
+ << *_collection->uuid()
+ << ")"};
+ }
+
invariant(_opCtx->lockState()->isNoop() || !_opCtx->lockState()->inAWriteUnitOfWork());
for (size_t i = 0; i < _indexes.size(); i++) {
if (_indexes[i].bulk == NULL)
@@ -536,10 +574,13 @@ Status MultiIndexBlockImpl::_doneInserting(std::set<RecordId>* dupRecords,
}
}
+ _setState(State::kPreCommit);
+
return Status::OK();
}
void MultiIndexBlockImpl::abortWithoutCleanup() {
+ _setStateToAbortedIfNotCommitted("aborted without cleanup"_sd);
_indexes.clear();
_needToCleanup = false;
}
@@ -549,6 +590,16 @@ Status MultiIndexBlockImpl::commit() {
}
Status MultiIndexBlockImpl::commit(stdx::function<void(const BSONObj& spec)> onCreateFn) {
+ if (State::kAborted == _getState()) {
+ return {ErrorCodes::IndexBuildAborted,
+ str::stream() << "Index build aborted: " << _abortReason
+ << ". Cannot commit index builder: "
+ << _collection->ns().ns()
+ << "("
+ << *_collection->uuid()
+ << ")"};
+ }
+
// Do not interfere with writing multikey information when committing index builds.
auto restartTracker =
MakeGuard([this] { MultikeyPathTracker::get(_opCtx).startTrackingMultikeyPathInfo(); });
@@ -582,10 +633,66 @@ Status MultiIndexBlockImpl::commit(stdx::function<void(const BSONObj& spec)> onC
}
}
+ // The state of this index build is set to Committed only when the WUOW commits.
+ // It is possible for abort() to be called after the check at the beginning of this function and
+ // before the WUOW is committed. If the WUOW commits, the final state of this index builder will
+ // be Committed. Otherwise, the index builder state will remain as Aborted and further attempts
+ // to commit this index build will fail.
_opCtx->recoveryUnit()->registerChange(new SetNeedToCleanupOnRollback(this));
+ _opCtx->recoveryUnit()->onCommit(
+ [this](boost::optional<Timestamp> commitTime) { this->_setState(State::kCommitted); });
_needToCleanup = false;
return Status::OK();
}
+bool MultiIndexBlockImpl::isCommitted() const {
+ return State::kCommitted == _getState();
+}
+
+void MultiIndexBlockImpl::abort(StringData reason) {
+ _setStateToAbortedIfNotCommitted(reason);
+}
+
+
+MultiIndexBlockImpl::State MultiIndexBlockImpl::getState_forTest() const {
+ return _getState();
+}
+
+MultiIndexBlockImpl::State MultiIndexBlockImpl::_getState() const {
+ stdx::lock_guard<stdx::mutex> lock(_mutex);
+ return _state;
+}
+
+void MultiIndexBlockImpl::_setState(State newState) {
+ invariant(State::kAborted != newState);
+ stdx::lock_guard<stdx::mutex> lock(_mutex);
+ _state = newState;
+}
+
+void MultiIndexBlockImpl::_setStateToAbortedIfNotCommitted(StringData reason) {
+ stdx::lock_guard<stdx::mutex> lock(_mutex);
+ if (State::kCommitted == _state) {
+ return;
+ }
+ _state = State::kAborted;
+ _abortReason = reason.toString();
+}
+
+std::ostream& operator<<(std::ostream& os, const MultiIndexBlockImpl::State& state) {
+ switch (state) {
+ case MultiIndexBlockImpl::State::kUninitialized:
+ return os << "Uninitialized";
+ case MultiIndexBlockImpl::State::kRunning:
+ return os << "Running";
+ case MultiIndexBlockImpl::State::kPreCommit:
+ return os << "PreCommit";
+ case MultiIndexBlockImpl::State::kCommitted:
+ return os << "Committed";
+ case MultiIndexBlockImpl::State::kAborted:
+ return os << "Aborted";
+ }
+ MONGO_UNREACHABLE;
+}
+
} // 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 b2a5c09110e..a1c7f726cba 100644
--- a/src/mongo/db/catalog/multi_index_block_impl.h
+++ b/src/mongo/db/catalog/multi_index_block_impl.h
@@ -31,6 +31,7 @@
#include "mongo/db/catalog/multi_index_block.h"
+#include <iosfwd>
#include <memory>
#include <set>
#include <string>
@@ -42,6 +43,7 @@
#include "mongo/db/catalog/index_catalog_impl.h"
#include "mongo/db/index/index_access_method.h"
#include "mongo/db/record_id.h"
+#include "mongo/stdx/mutex.h"
namespace mongo {
@@ -90,12 +92,37 @@ public:
Status commit() override;
Status commit(stdx::function<void(const BSONObj& spec)> onCreateFn) override;
+ bool isCommitted() const override;
+
+ void abort(StringData reason) override;
+
void abortWithoutCleanup() override;
bool getBuildInBackground() const override {
return _buildInBackground;
}
+ /**
+ * State transitions:
+ *
+ * Uninitialized --> Running --> PreCommit --> Committed
+ * | | | ^
+ * | | | |
+ * \--------------+------------+-------> Aborted
+ *
+ * It is possible for abort() to skip intermediate states. For example, calling abort() when the
+ * index build has not been initialized will transition from Uninitialized directly to Aborted.
+ *
+ * In the case where we are in the midst of committing the WUOW for a successful commit() call,
+ * we may transition temporarily to Aborted before finally ending at Committed. See comments for
+ * MultiIndexBlock::abort().
+ *
+ * Not part of MultiIndexBlock interface. Callers should not have to query the state of the
+ * MultiIndexBlock directly.
+ */
+ enum class State { kUninitialized, kRunning, kPreCommit, kCommitted, kAborted };
+ State getState_forTest() const;
+
private:
class SetNeedToCleanupOnRollback;
class CleanupIndexesVectorOnRollback;
@@ -112,6 +139,21 @@ private:
Status _doneInserting(std::set<RecordId>* dupRecords, std::vector<BSONObj>* dupKeysInserted);
+ /**
+ * Returns the current state.
+ */
+ State _getState() const;
+
+ /**
+ * Updates the current state to a non-Aborted state.
+ */
+ void _setState(State newState);
+
+ /**
+ * Updates the current state to Aborted with the given reason.
+ */
+ void _setStateToAbortedIfNotCommitted(StringData reason);
+
std::vector<IndexToBuild> _indexes;
std::unique_ptr<BackgroundOperation> _backgroundOperation;
@@ -125,6 +167,16 @@ private:
bool _ignoreUnique;
bool _needToCleanup;
+
+ // Protects member variables of this class declared below.
+ mutable stdx::mutex _mutex;
+
+ State _state = State::kUninitialized;
+ std::string _abortReason;
};
+// For unit tests that need to check MultiIndexBlock states.
+// The ASSERT_*() macros use this function to print the value of 'state' when the predicate fails.
+std::ostream& operator<<(std::ostream& os, const MultiIndexBlockImpl::State& state);
+
} // namespace mongo
diff --git a/src/mongo/db/catalog/multi_index_block_test.cpp b/src/mongo/db/catalog/multi_index_block_test.cpp
index 4f09562a847..11dcd74502c 100644
--- a/src/mongo/db/catalog/multi_index_block_test.cpp
+++ b/src/mongo/db/catalog/multi_index_block_test.cpp
@@ -28,29 +28,211 @@
* it in the license file.
*/
-#include "mongo/db/catalog/multi_index_block.h"
-
-#include "mongo/db/catalog/collection.h"
#include "mongo/db/catalog/collection_mock.h"
+#include "mongo/db/catalog/index_catalog_noop.h"
#include "mongo/db/catalog/multi_index_block_impl.h"
-#include "mongo/db/operation_context_noop.h"
+#include "mongo/db/jsobj.h"
+#include "mongo/db/repl/replication_coordinator_mock.h"
+#include "mongo/db/service_context_test_fixture.h"
#include "mongo/unittest/unittest.h"
namespace mongo {
namespace {
/**
- * Simple initialization test for MultiIndexBlock to check library dependencies.
- * For greater test coverage, it may be necessary to make this test fixture inherit from
+ * Unit test for MultiIndexBlock to verify basic functionality.
+ * Using a mocked Collection object ensures that we are pulling in a minimal set of library
+ * dependencies.
+ * For integration tests, it may be necessary to make this test fixture inherit from
* ServiceContextMongoDTest.
*/
-class MultiIndexBlockTest : public unittest::Test {};
+class MultiIndexBlockTest : public ServiceContextTest {
+private:
+ void setUp() override;
+ void tearDown() override;
+
+protected:
+ OperationContext* getOpCtx() const;
+ Collection* getCollection() const;
+ MultiIndexBlockImpl* getIndexer() const;
+
+private:
+ ServiceContext::UniqueOperationContext _opCtx;
+ std::unique_ptr<Collection> _collection;
+ std::unique_ptr<MultiIndexBlockImpl> _indexer;
+};
+
+void MultiIndexBlockTest::setUp() {
+ ServiceContextTest::setUp();
+
+ auto service = getServiceContext();
+ repl::ReplicationCoordinator::set(service,
+ std::make_unique<repl::ReplicationCoordinatorMock>(service));
+
+ _opCtx = makeOperationContext();
-TEST_F(MultiIndexBlockTest, Create) {
- OperationContextNoop opCtx;
NamespaceString nss("mydb.mycoll");
- Collection collection(std::make_unique<CollectionMock>(nss));
- MultiIndexBlockImpl(&opCtx, &collection);
+ auto collectionMock =
+ std::make_unique<CollectionMock>(nss, std::make_unique<IndexCatalogNoop>());
+ _collection = std::make_unique<Collection>(std::move(collectionMock));
+
+ _indexer = std::make_unique<MultiIndexBlockImpl>(_opCtx.get(), _collection.get());
+}
+
+void MultiIndexBlockTest::tearDown() {
+ auto service = getServiceContext();
+ repl::ReplicationCoordinator::set(service, {});
+
+ _indexer = {};
+
+ _collection = {};
+
+ _opCtx = {};
+
+ ServiceContextTest::tearDown();
+}
+
+OperationContext* MultiIndexBlockTest::getOpCtx() const {
+ return _opCtx.get();
+}
+
+Collection* MultiIndexBlockTest::getCollection() const {
+ return _collection.get();
+}
+
+MultiIndexBlockImpl* MultiIndexBlockTest::getIndexer() const {
+ return _indexer.get();
+}
+
+TEST_F(MultiIndexBlockTest, CommitWithoutInsertingDocuments) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ ASSERT_OK(indexer->doneInserting());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kPreCommit, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+ {
+ WriteUnitOfWork wunit(getOpCtx());
+ ASSERT_OK(indexer->commit());
+ wunit.commit();
+ }
+ ASSERT(indexer->isCommitted());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kCommitted, indexer->getState_forTest());
+}
+
+TEST_F(MultiIndexBlockTest, CommitAfterInsertingSingleDocument) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ ASSERT_OK(indexer->insert({}, {}, nullptr));
+ ASSERT_OK(indexer->doneInserting());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kPreCommit, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+ {
+ WriteUnitOfWork wunit(getOpCtx());
+ ASSERT_OK(indexer->commit());
+ wunit.commit();
+ }
+ ASSERT(indexer->isCommitted());
+
+ // abort() should have no effect after the index build is committed.
+ indexer->abort("test"_sd);
+ ASSERT(indexer->isCommitted());
+}
+
+TEST_F(MultiIndexBlockTest, AbortWithoutCleanupAfterInsertingSingleDocument) {
+ auto indexer = getIndexer();
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_OK(indexer->insert({}, {}, nullptr));
+ indexer->abortWithoutCleanup();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+}
+
+TEST_F(MultiIndexBlockTest, InitFailsAfterAbort) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ indexer->abort("test"_sd);
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_EQUALS(ErrorCodes::IndexBuildAborted, indexer->init(std::vector<BSONObj>()).getStatus());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+}
+
+TEST_F(MultiIndexBlockTest, InsertingSingleDocumentFailsAfterAbort) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ indexer->abort("test"_sd);
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_EQUALS(ErrorCodes::IndexBuildAborted,
+ indexer->insert(BSON("_id" << 123 << "a" << 456), {}, nullptr));
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+}
+
+TEST_F(MultiIndexBlockTest, DoneInsertingFailsAfterAbort) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ ASSERT_OK(indexer->insert(BSON("_id" << 123 << "a" << 456), {}, nullptr));
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ indexer->abort("test"_sd);
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_EQUALS(ErrorCodes::IndexBuildAborted, indexer->doneInserting());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
+}
+
+TEST_F(MultiIndexBlockTest, CommitFailsAfterAbort) {
+ auto indexer = getIndexer();
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kUninitialized, indexer->getState_forTest());
+
+ auto specs = unittest::assertGet(indexer->init(std::vector<BSONObj>()));
+ ASSERT_EQUALS(0U, specs.size());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ ASSERT_OK(indexer->insert(BSON("_id" << 123 << "a" << 456), {}, nullptr));
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kRunning, indexer->getState_forTest());
+
+ ASSERT_OK(indexer->doneInserting());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kPreCommit, indexer->getState_forTest());
+
+ indexer->abort("test"_sd);
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_EQUALS(ErrorCodes::IndexBuildAborted, indexer->commit());
+ ASSERT_EQUALS(MultiIndexBlockImpl::State::kAborted, indexer->getState_forTest());
+
+ ASSERT_FALSE(indexer->isCommitted());
}
} // namespace