diff options
author | Louis Williams <louis.williams@mongodb.com> | 2020-07-01 10:09:51 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-07-01 15:55:15 +0000 |
commit | e4f92ef12f233e86be9de019f16db4f5dde47ad5 (patch) | |
tree | 2460e5c14d70d25d6625b86ee56978a9b38af487 /src/mongo/db | |
parent | 9f4b81e5bdcf38f9b10459203a804ba406528770 (diff) | |
download | mongo-e4f92ef12f233e86be9de019f16db4f5dde47ad5.tar.gz |
SERVER-47867 Remove ghost timestamping code
Ensures all writes to the durable catalog are timestamped when required.
deleted: src/mongo/db/catalog/index_timestamp_helper.cpp
deleted: src/mongo/db/catalog/index_timestamp_helper.h
Diffstat (limited to 'src/mongo/db')
18 files changed, 216 insertions, 339 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 81397c5c5d3..28ee78de758 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -852,7 +852,6 @@ env.Library( 'index_build_entry_helpers', '$BUILD_DIR/mongo/db/catalog/collection_catalog', '$BUILD_DIR/mongo/db/catalog/index_build_entry_idl', - '$BUILD_DIR/mongo/db/catalog/index_timestamp_helper', "$BUILD_DIR/mongo/executor/task_executor_interface", "$BUILD_DIR/mongo/db/storage/two_phase_index_build_knobs_idl", ], diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript index c94140d876f..0178b45e9a8 100644 --- a/src/mongo/db/catalog/SConscript +++ b/src/mongo/db/catalog/SConscript @@ -178,22 +178,6 @@ env.Library( ) env.Library( - target='index_timestamp_helper', - source=[ - "index_timestamp_helper.cpp", - ], - LIBDEPS=[ - '$BUILD_DIR/mongo/base', - ], - LIBDEPS_PRIVATE=[ - '$BUILD_DIR/mongo/db/concurrency/write_conflict_exception', - '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', - '$BUILD_DIR/mongo/db/server_options_core', - '$BUILD_DIR/mongo/db/vector_clock', - ] -) - -env.Library( target='index_build_block', source=[ "index_build_block.cpp", @@ -252,7 +236,6 @@ env.Library( '$BUILD_DIR/mongo/util/progress_meter', 'collection', 'index_build_block', - 'index_timestamp_helper', ], LIBDEPS_PRIVATE=[ '$BUILD_DIR/mongo/db/index/index_build_interceptor', diff --git a/src/mongo/db/catalog/index_builds_manager.cpp b/src/mongo/db/catalog/index_builds_manager.cpp index d6e379bce7f..1548bf51850 100644 --- a/src/mongo/db/catalog/index_builds_manager.cpp +++ b/src/mongo/db/catalog/index_builds_manager.cpp @@ -36,7 +36,6 @@ #include "mongo/db/catalog/collection.h" #include "mongo/db/catalog/collection_catalog.h" #include "mongo/db/catalog/index_catalog.h" -#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/catalog/uncommitted_collections.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/operation_context.h" diff --git a/src/mongo/db/catalog/index_timestamp_helper.cpp b/src/mongo/db/catalog/index_timestamp_helper.cpp deleted file mode 100644 index 5da115be516..00000000000 --- a/src/mongo/db/catalog/index_timestamp_helper.cpp +++ /dev/null @@ -1,140 +0,0 @@ -/** - * Copyright (C) 2019-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kIndex - -#include "mongo/db/catalog/index_timestamp_helper.h" - -#include "mongo/db/concurrency/write_conflict_exception.h" -#include "mongo/db/logical_clock.h" -#include "mongo/db/repl/member_state.h" -#include "mongo/db/repl/replication_coordinator.h" -#include "mongo/logv2/log.h" - -namespace mongo { - -namespace { -Status setGhostTimestamp(OperationContext* opCtx, Timestamp timestamp) { - if (auto status = opCtx->recoveryUnit()->setTimestamp(timestamp); !status.isOK()) { - return status; - } - opCtx->recoveryUnit()->setOrderedCommit(false); - - auto replCoord = repl::ReplicationCoordinator::get(opCtx); - if (replCoord->isReplEnabled()) { - opCtx->recoveryUnit()->onCommit( - [replCoord](auto commitTime) { replCoord->attemptToAdvanceStableTimestamp(); }); - } - - return Status::OK(); -} - -bool requiresGhostCommitTimestampForWrite(OperationContext* opCtx, const NamespaceString& nss) { - if (!nss.isReplicated()) { - return false; - } - - auto replCoord = repl::ReplicationCoordinator::get(opCtx); - if (!replCoord->isReplEnabled()) { - return false; - } - - if (replCoord->getMemberState().startup2()) { - return false; - } - - // Only storage engines that support recover-to-stable need ghost commit timestamps. - if (!opCtx->getServiceContext()->getStorageEngine()->supportsRecoveryTimestamp()) { - return false; - } - - return true; -} -} // namespace - -bool IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, - NamespaceString nss) { - if (opCtx->writesAreReplicated()) { - return false; - } - - if (!nss.isReplicated() || nss.coll().startsWith("tmp.mr.")) { - return false; - } - - auto replCoord = repl::ReplicationCoordinator::get(opCtx); - if (!replCoord->isReplEnabled()) { - return false; - } - - // If there is a commit timestamp already assigned, there's no need to explicitly assign a - // timestamp. This case covers foreground index builds. - if (!opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { - return false; - } - - // Only oplog entries (including a user's `applyOps` command) construct indexes via - // `IndexBuilder`. Nodes in `startup` may not yet have initialized the `LogicalClock`, however - // index builds during startup replication recovery must be timestamped. These index builds - // are foregrounded and timestamp their catalog writes with a "commit timestamp". Nodes in the - // oplog application phase of initial sync (`startup2`) must not timestamp index builds before - // the `initialDataTimestamp`. - const auto memberState = replCoord->getMemberState(); - if (memberState.startup() || memberState.startup2()) { - return false; - } - - // When in rollback via refetch, it's valid for all writes to be untimestamped. Additionally, - // it's illegal to timestamp a write later than the timestamp associated with the node exiting - // the rollback state. This condition opts for being conservative. - if (!serverGlobalParams.enableMajorityReadConcern && memberState.rollback()) { - return false; - } - - return true; -} - -bool IndexTimestampHelper::setGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, - const NamespaceString& nss) { - invariant(opCtx->lockState()->inAWriteUnitOfWork()); - if (!requiresGhostCommitTimestampForCatalogWrite(opCtx, nss)) { - return false; - } - auto status = - setGhostTimestamp(opCtx, LogicalClock::get(opCtx)->getClusterTime().asTimestamp()); - if (status.code() == ErrorCodes::BadValue) { - LOGV2(20381, - "Temporarily could not timestamp the index build commit, retrying", - "reason"_attr = status.reason()); - throw WriteConflictException(); - } - fassert(50701, status); - return true; -} -} // namespace mongo diff --git a/src/mongo/db/catalog/index_timestamp_helper.h b/src/mongo/db/catalog/index_timestamp_helper.h deleted file mode 100644 index 938f7d14f57..00000000000 --- a/src/mongo/db/catalog/index_timestamp_helper.h +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Copyright (C) 2019-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#pragma once - -#include "mongo/db/namespace_string.h" -#include "mongo/db/operation_context.h" - -namespace mongo { - -namespace IndexTimestampHelper { - -/** - * Returns true if writes to the catalog entry for the input namespace require being - * timestamped. A ghost write is when the operation is not committed with an oplog entry and - * implies the caller will look at the logical clock to choose a time to use. - */ -bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, NamespaceString nss); - -/** - * If required, sets a timestamp on an active WriteUnitOfWork for a catalog write. A ghost write - * is when the operation is not committed with an oplog entry, which may be necessary for - * certain index catalog operations not associated with a unique optime. This implementation - * uses the LogicalClock to timestamp operations. - * Returns true if a ghost timestamp was set, false if no timestamp was required to be set. Can - * also throw WriteConflictException. - */ -bool setGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, const NamespaceString& nss); -}; // namespace IndexTimestampHelper - -} // namespace mongo diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp index 8334a9506a9..9a406a06e77 100644 --- a/src/mongo/db/catalog/multi_index_block.cpp +++ b/src/mongo/db/catalog/multi_index_block.cpp @@ -39,7 +39,6 @@ #include "mongo/db/audit.h" #include "mongo/db/catalog/collection.h" #include "mongo/db/catalog/collection_catalog.h" -#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/catalog/multi_index_block_gen.h" #include "mongo/db/catalog/uncommitted_collections.h" #include "mongo/db/client.h" @@ -104,22 +103,6 @@ void MultiIndexBlock::abortIndexBuild(OperationContext* opCtx, opCtx, TemporaryRecordStore::FinalizationAction::kDelete); } - // Nodes building an index on behalf of a user (e.g: `createIndexes`, `applyOps`) may - // fail, removing the existence of the index from the catalog. This update must be - // timestamped (unless the build is on an unreplicated collection). A failure from - // `createIndexes` should not have a commit timestamp and instead write a noop entry. A - // foreground `applyOps` index build may have a commit timestamp already set. - if (opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { - // We must choose a timestamp to write with, as we don't have one handy in the - // recovery unit already. - - // Simply get a timestamp to write with here; we can't write to the oplog. - repl::UnreplicatedWritesBlock uwb(opCtx); - if (!IndexTimestampHelper::setGhostCommitTimestampForCatalogWrite(opCtx, nss)) { - LOGV2(20382, "Did not timestamp index abort write"); - } - } - onCleanUp(); wunit.commit(); diff --git a/src/mongo/db/index/SConscript b/src/mongo/db/index/SConscript index 84f2727d177..922242cb618 100644 --- a/src/mongo/db/index/SConscript +++ b/src/mongo/db/index/SConscript @@ -176,7 +176,6 @@ env.Library( 'skipped_record_tracker', ], LIBDEPS_PRIVATE=[ - '$BUILD_DIR/mongo/db/catalog/index_timestamp_helper', '$BUILD_DIR/mongo/db/multi_key_path_tracker', 'index_access_methods', ], diff --git a/src/mongo/db/index/index_build_interceptor.cpp b/src/mongo/db/index/index_build_interceptor.cpp index 1c93f6dadc4..8d319e51536 100644 --- a/src/mongo/db/index/index_build_interceptor.cpp +++ b/src/mongo/db/index/index_build_interceptor.cpp @@ -36,7 +36,6 @@ #include <vector> #include "mongo/bson/bsonobj.h" -#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/catalog_raii.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/curop.h" diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 826ec658618..c0b121bb0f1 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -37,7 +37,6 @@ #include "mongo/db/catalog/commit_quorum_options.h" #include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/index_build_entry_gen.h" -#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/catalog/uncommitted_collections.h" #include "mongo/db/catalog_raii.h" #include "mongo/db/concurrency/locker.h" diff --git a/src/mongo/db/storage/SConscript b/src/mongo/db/storage/SConscript index 740e673bd28..fe02bdadf11 100644 --- a/src/mongo/db/storage/SConscript +++ b/src/mongo/db/storage/SConscript @@ -487,8 +487,8 @@ env.Library( '$BUILD_DIR/mongo/db/catalog/collection_catalog', ], LIBDEPS_PRIVATE=[ - '$BUILD_DIR/mongo/db/catalog/index_timestamp_helper', '$BUILD_DIR/mongo/db/concurrency/write_conflict_exception', + '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', '$BUILD_DIR/mongo/db/server_options_core', ], ) diff --git a/src/mongo/db/storage/biggie/SConscript b/src/mongo/db/storage/biggie/SConscript index d4fd98e0699..7326768141a 100644 --- a/src/mongo/db/storage/biggie/SConscript +++ b/src/mongo/db/storage/biggie/SConscript @@ -55,8 +55,11 @@ env.CppUnitTest( ], LIBDEPS=[ 'storage_biggie_core', + '$BUILD_DIR/mongo/db/auth/authmocks', '$BUILD_DIR/mongo/db/common', '$BUILD_DIR/mongo/db/index/index_descriptor', + '$BUILD_DIR/mongo/db/repl/replmocks', + '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', '$BUILD_DIR/mongo/db/storage/key_string', '$BUILD_DIR/mongo/db/storage/kv/kv_engine_test_harness', '$BUILD_DIR/mongo/db/storage/record_store_test_harness', diff --git a/src/mongo/db/storage/biggie/biggie_kv_engine_test.cpp b/src/mongo/db/storage/biggie/biggie_kv_engine_test.cpp index dac1a579a2a..fb75b79f7b3 100644 --- a/src/mongo/db/storage/biggie/biggie_kv_engine_test.cpp +++ b/src/mongo/db/storage/biggie/biggie_kv_engine_test.cpp @@ -34,6 +34,7 @@ #include <memory> #include "mongo/base/init.h" +#include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/service_context.h" #include "mongo/db/service_context_test_fixture.h" #include "mongo/db/storage/biggie/biggie_kv_engine.h" @@ -47,6 +48,10 @@ public: BiggieKVHarnessHelper() { invariant(hasGlobalServiceContext()); _engine = std::make_unique<KVEngine>(); + repl::ReplicationCoordinator::set( + getGlobalServiceContext(), + std::unique_ptr<repl::ReplicationCoordinator>(new repl::ReplicationCoordinatorMock( + getGlobalServiceContext(), repl::ReplSettings()))); } virtual KVEngine* getEngine() override { diff --git a/src/mongo/db/storage/durable_catalog_impl.cpp b/src/mongo/db/storage/durable_catalog_impl.cpp index a327c16cce1..b5f1c526cdf 100644 --- a/src/mongo/db/storage/durable_catalog_impl.cpp +++ b/src/mongo/db/storage/durable_catalog_impl.cpp @@ -37,12 +37,12 @@ #include "mongo/bson/util/bson_extract.h" #include "mongo/bson/util/builder.h" -#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context.h" +#include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/server_options.h" #include "mongo/db/storage/durable_catalog_feature_tracker.h" #include "mongo/db/storage/kv/kv_engine.h" @@ -140,6 +140,46 @@ bool indexTypeSupportsPathLevelMultikeyTracking(StringData accessMethod) { return accessMethod == IndexNames::BTREE || accessMethod == IndexNames::GEO_2DSPHERE; } +/** + * Returns true if writes to the catalog entry for the input namespace require being timestamped. + */ +bool requiresTimestampForCatalogWrite(OperationContext* opCtx, const NamespaceString& nss) { + if (!nss.isReplicated() || nss.coll().startsWith("tmp.mr.") || nss.isDropPendingNamespace()) { + return false; + } + + auto replCoord = repl::ReplicationCoordinator::get(opCtx); + if (!replCoord->isReplEnabled()) { + return false; + } + + if (replCoord->canAcceptWritesFor(opCtx, nss)) { + return false; + } + + // If there is a timestamp already assigned, there's no need to explicitly assign a timestamp. + if (opCtx->recoveryUnit()->isTimestamped()) { + return false; + } + + // Nodes in `startup` do not need to timestamp writes. + // Nodes in the oplog application phase of initial sync (`startup2`) must not timestamp writes + // before the `initialDataTimestamp`. + const auto memberState = replCoord->getMemberState(); + if (memberState.startup() || memberState.startup2()) { + return false; + } + + // When rollback completes, index builds may be restarted, which requires untimestamped catalog + // writes. Additionally, it's illegal to timestamp a write later than the timestamp associated + // with the node exiting the rollback state. + if (memberState.rollback()) { + return false; + } + + return true; +} + } // namespace using std::string; @@ -634,8 +674,7 @@ void DurableCatalogImpl::putMetaData(OperationContext* opCtx, obj = b.obj(); } - if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, nss) && - !nss.isDropPendingNamespace()) { + if (requiresTimestampForCatalogWrite(opCtx, nss)) { opCtx->recoveryUnit()->setMustBeTimestamped(); } @@ -681,9 +720,7 @@ Status DurableCatalogImpl::_replaceEntry(OperationContext* opCtx, it->second.nss = fromName; }); - NamespaceString fromNss(fromName); - if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, fromNss) && - !fromNss.isDropPendingNamespace()) { + if (requiresTimestampForCatalogWrite(opCtx, fromName)) { opCtx->recoveryUnit()->setMustBeTimestamped(); } diff --git a/src/mongo/db/storage/ephemeral_for_test/SConscript b/src/mongo/db/storage/ephemeral_for_test/SConscript index d2f9760eec7..966fd417d77 100644 --- a/src/mongo/db/storage/ephemeral_for_test/SConscript +++ b/src/mongo/db/storage/ephemeral_for_test/SConscript @@ -67,5 +67,7 @@ env.CppUnitTest( ], LIBDEPS_PRIVATE=[ '$BUILD_DIR/mongo/db/auth/authmocks', + '$BUILD_DIR/mongo/db/repl/replmocks', + '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', ], ) diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_engine_test.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_engine_test.cpp index ffdc4adee09..7e562dac4e5 100644 --- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_engine_test.cpp +++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_engine_test.cpp @@ -32,14 +32,22 @@ #include <memory> #include "mongo/base/init.h" +#include "mongo/db/repl/replication_coordinator_mock.h" +#include "mongo/db/service_context_test_fixture.h" #include "mongo/db/storage/kv/kv_engine_test_harness.h" namespace mongo { namespace { -class EphemeralForTestKVHarnessHelper : public KVHarnessHelper { +class EphemeralForTestKVHarnessHelper : public KVHarnessHelper, + public ScopedGlobalServiceContextForTest { public: - EphemeralForTestKVHarnessHelper() : _engine(new EphemeralForTestEngine()) {} + EphemeralForTestKVHarnessHelper() : _engine(new EphemeralForTestEngine()) { + repl::ReplicationCoordinator::set( + getGlobalServiceContext(), + std::unique_ptr<repl::ReplicationCoordinator>(new repl::ReplicationCoordinatorMock( + getGlobalServiceContext(), repl::ReplSettings()))); + } virtual KVEngine* restartEngine() { // Intentionally not restarting since the in-memory storage engine diff --git a/src/mongo/db/storage/kv/durable_catalog_test.cpp b/src/mongo/db/storage/kv/durable_catalog_test.cpp index 5710f1a191c..31a58b404dc 100644 --- a/src/mongo/db/storage/kv/durable_catalog_test.cpp +++ b/src/mongo/db/storage/kv/durable_catalog_test.cpp @@ -39,6 +39,7 @@ #include "mongo/db/index/multikey_paths.h" #include "mongo/db/index_names.h" #include "mongo/db/operation_context_noop.h" +#include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/service_context_test_fixture.h" #include "mongo/db/storage/devnull/devnull_kv_engine.h" #include "mongo/db/storage/kv/kv_engine.h" @@ -62,6 +63,10 @@ public: : _nss("unittests.durable_catalog"), _storageEngine(std::make_unique<DevNullKVEngine>(), StorageEngineOptions()) { _storageEngine.finishInit(); + repl::ReplicationCoordinator::set( + getGlobalServiceContext(), + std::unique_ptr<repl::ReplicationCoordinator>(new repl::ReplicationCoordinatorMock( + getGlobalServiceContext(), repl::ReplSettings()))); } ~DurableCatalogTest() { diff --git a/src/mongo/db/storage/kv/kv_engine_test_harness.cpp b/src/mongo/db/storage/kv/kv_engine_test_harness.cpp index de95507fc6c..4195bddcf5d 100644 --- a/src/mongo/db/storage/kv/kv_engine_test_harness.cpp +++ b/src/mongo/db/storage/kv/kv_engine_test_harness.cpp @@ -45,8 +45,39 @@ namespace mongo { +class ClientAndCtx { +public: + ClientAndCtx(ServiceContext::UniqueClient client, ServiceContext::UniqueOperationContext opCtx) + : _client(std::move(client)), _opCtx(std::move(opCtx)) {} + + OperationContext* opCtx() { + return _opCtx.get(); + } + + Client* client() { + return _client.get(); + } + + ServiceContext::UniqueClient _client; + ServiceContext::UniqueOperationContext _opCtx; +}; + class DurableCatalogImplTest : public unittest::Test { protected: + void setUp() override { + helper = KVHarnessHelper::create(); + invariant(hasGlobalServiceContext()); + } + + ClientAndCtx makeClientAndCtx(const std::string& clientName) { + auto client = getGlobalServiceContext()->makeClient(clientName); + auto opCtx = client->makeOperationContext(); + opCtx->setRecoveryUnit( + std::unique_ptr<RecoveryUnit>(helper->getEngine()->newRecoveryUnit()), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + return {std::move(client), std::move(opCtx)}; + } + RecordId newCollection(OperationContext* opCtx, const NamespaceString& ns, const CollectionOptions& options, @@ -70,6 +101,8 @@ protected: DurableCatalogImpl* catalog) { return catalog->_removeEntry(opCtx, catalogId); } + + std::unique_ptr<KVHarnessHelper> helper; }; namespace { @@ -167,7 +200,6 @@ TEST(KVEngineTestHarness, Restart1) { TEST(KVEngineTestHarness, SimpleSorted1) { - setGlobalServiceContext(ServiceContext::make()); std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); ASSERT(engine); @@ -333,25 +365,26 @@ TEST(KVEngineTestHarness, AllDurableTimestamp) { } TEST_F(DurableCatalogImplTest, Coll1) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); std::unique_ptr<RecordStore> rs; std::unique_ptr<DurableCatalogImpl> catalog; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), false, false, nullptr); uow.commit(); } RecordId catalogId; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - catalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + catalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -362,20 +395,22 @@ TEST_F(DurableCatalogImplTest, Coll1) { std::string ident = catalog->getEntry(catalogId).ident; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), false, false, nullptr); - catalog->init(&opCtx); + catalog->init(opCtx); uow.commit(); } ASSERT_EQUALS(ident, catalog->getEntry(catalogId).ident); RecordId newCatalogId; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - dropCollection(&opCtx, catalogId, catalog.get()).transitional_ignore(); - newCatalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + dropCollection(opCtx, catalogId, catalog.get()).transitional_ignore(); + newCatalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -386,25 +421,26 @@ TEST_F(DurableCatalogImplTest, Coll1) { } TEST_F(DurableCatalogImplTest, Idx1) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); std::unique_ptr<RecordStore> rs; std::unique_ptr<DurableCatalogImpl> catalog; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), false, false, nullptr); uow.commit(); } RecordId catalogId; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - catalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + catalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -415,8 +451,9 @@ TEST_F(DurableCatalogImplTest, Idx1) { } { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); BSONCollectionCatalogEntry::MetaData md; md.ns = "a.b"; @@ -429,29 +466,32 @@ TEST_F(DurableCatalogImplTest, Idx1) { imd.prefix = KVPrefix::kNotPrefixed; imd.isBackgroundSecondaryBuild = false; md.indexes.push_back(imd); - catalog->putMetaData(&opCtx, catalogId, md); + catalog->putMetaData(opCtx, catalogId, md); uow.commit(); } std::string idxIndent; { - MyOperationContext opCtx(engine); - idxIndent = catalog->getIndexIdent(&opCtx, catalogId, "foo"); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + idxIndent = catalog->getIndexIdent(opCtx, catalogId, "foo"); } { - MyOperationContext opCtx(engine); - ASSERT_EQUALS(idxIndent, catalog->getIndexIdent(&opCtx, catalogId, "foo")); - ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(&opCtx, catalogId, "foo"))); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + ASSERT_EQUALS(idxIndent, catalog->getIndexIdent(opCtx, catalogId, "foo")); + ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(opCtx, catalogId, "foo"))); } { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); BSONCollectionCatalogEntry::MetaData md; md.ns = "a.b"; - catalog->putMetaData(&opCtx, catalogId, md); // remove index + catalog->putMetaData(opCtx, catalogId, md); // remove index BSONCollectionCatalogEntry::IndexMetaData imd; imd.spec = BSON("name" @@ -461,36 +501,38 @@ TEST_F(DurableCatalogImplTest, Idx1) { imd.prefix = KVPrefix::kNotPrefixed; imd.isBackgroundSecondaryBuild = false; md.indexes.push_back(imd); - catalog->putMetaData(&opCtx, catalogId, md); + catalog->putMetaData(opCtx, catalogId, md); uow.commit(); } { - MyOperationContext opCtx(engine); - ASSERT_NOT_EQUALS(idxIndent, catalog->getIndexIdent(&opCtx, catalogId, "foo")); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + ASSERT_NOT_EQUALS(idxIndent, catalog->getIndexIdent(opCtx, catalogId, "foo")); } } TEST_F(DurableCatalogImplTest, DirectoryPerDb1) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); std::unique_ptr<RecordStore> rs; std::unique_ptr<DurableCatalogImpl> catalog; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), true, false, nullptr); uow.commit(); } RecordId catalogId; { // collection - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - catalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + catalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -501,8 +543,9 @@ TEST_F(DurableCatalogImplTest, DirectoryPerDb1) { } { // index - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); BSONCollectionCatalogEntry::MetaData md; md.ns = "a.b"; @@ -515,33 +558,34 @@ TEST_F(DurableCatalogImplTest, DirectoryPerDb1) { imd.prefix = KVPrefix::kNotPrefixed; imd.isBackgroundSecondaryBuild = false; md.indexes.push_back(imd); - catalog->putMetaData(&opCtx, catalogId, md); - ASSERT_STRING_CONTAINS(catalog->getIndexIdent(&opCtx, catalogId, "foo"), "a/"); - ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(&opCtx, catalogId, "foo"))); + catalog->putMetaData(opCtx, catalogId, md); + ASSERT_STRING_CONTAINS(catalog->getIndexIdent(opCtx, catalogId, "foo"), "a/"); + ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(opCtx, catalogId, "foo"))); uow.commit(); } } TEST_F(DurableCatalogImplTest, Split1) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); std::unique_ptr<RecordStore> rs; std::unique_ptr<DurableCatalogImpl> catalog; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), false, true, nullptr); uow.commit(); } RecordId catalogId; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - catalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + catalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -552,8 +596,9 @@ TEST_F(DurableCatalogImplTest, Split1) { } { // index - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); BSONCollectionCatalogEntry::MetaData md; md.ns = "a.b"; @@ -566,33 +611,34 @@ TEST_F(DurableCatalogImplTest, Split1) { imd.prefix = KVPrefix::kNotPrefixed; imd.isBackgroundSecondaryBuild = false; md.indexes.push_back(imd); - catalog->putMetaData(&opCtx, catalogId, md); - ASSERT_STRING_CONTAINS(catalog->getIndexIdent(&opCtx, catalogId, "foo"), "index/"); - ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(&opCtx, catalogId, "foo"))); + catalog->putMetaData(opCtx, catalogId, md); + ASSERT_STRING_CONTAINS(catalog->getIndexIdent(opCtx, catalogId, "foo"), "index/"); + ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(opCtx, catalogId, "foo"))); uow.commit(); } } TEST_F(DurableCatalogImplTest, DirectoryPerAndSplit1) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); std::unique_ptr<RecordStore> rs; std::unique_ptr<DurableCatalogImpl> catalog; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); catalog = std::make_unique<DurableCatalogImpl>(rs.get(), true, true, nullptr); uow.commit(); } RecordId catalogId; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - catalogId = newCollection(&opCtx, + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + catalogId = newCollection(opCtx, NamespaceString("a.b"), CollectionOptions(), KVPrefix::kNotPrefixed, @@ -603,8 +649,9 @@ TEST_F(DurableCatalogImplTest, DirectoryPerAndSplit1) { } { // index - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); BSONCollectionCatalogEntry::MetaData md; md.ns = "a.b"; @@ -617,30 +664,28 @@ TEST_F(DurableCatalogImplTest, DirectoryPerAndSplit1) { imd.prefix = KVPrefix::kNotPrefixed; imd.isBackgroundSecondaryBuild = false; md.indexes.push_back(imd); - catalog->putMetaData(&opCtx, catalogId, md); - ASSERT_STRING_CONTAINS(catalog->getIndexIdent(&opCtx, catalogId, "foo"), "a/index/"); - ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(&opCtx, catalogId, "foo"))); + catalog->putMetaData(opCtx, catalogId, md); + ASSERT_STRING_CONTAINS(catalog->getIndexIdent(opCtx, catalogId, "foo"), "a/index/"); + ASSERT_TRUE(catalog->isUserDataIdent(catalog->getIndexIdent(opCtx, catalogId, "foo"))); uow.commit(); } } TEST_F(DurableCatalogImplTest, BackupImplemented) { - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); ASSERT(engine); { - MyOperationContext opCtx(engine); - ASSERT_OK(engine->beginBackup(&opCtx)); - engine->endBackup(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + ASSERT_OK(engine->beginBackup(opCtx)); + engine->endBackup(opCtx); } } DEATH_TEST_REGEX_F(DurableCatalogImplTest, TerminateOnNonNumericIndexVersion, "Fatal assertion.*50942") { - setGlobalServiceContext(ServiceContext::make()); - std::unique_ptr<KVHarnessHelper> helper(KVHarnessHelper::create()); KVEngine* engine = helper->getEngine(); ASSERT(engine); @@ -649,19 +694,21 @@ DEATH_TEST_REGEX_F(DurableCatalogImplTest, std::unique_ptr<RecordStore> rs; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); - ASSERT_OK(engine->createRecordStore(&opCtx, "catalog", "catalog", CollectionOptions())); - rs = engine->getRecordStore(&opCtx, "catalog", "catalog", CollectionOptions()); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); + ASSERT_OK(engine->createRecordStore(opCtx, "catalog", "catalog", CollectionOptions())); + rs = engine->getRecordStore(opCtx, "catalog", "catalog", CollectionOptions()); uow.commit(); } std::unique_ptr<CollectionImpl> collection; { - MyOperationContext opCtx(engine); - WriteUnitOfWork uow(&opCtx); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + WriteUnitOfWork uow(opCtx); collection = - std::make_unique<CollectionImpl>(&opCtx, ns, RecordId(0), UUID::gen(), std::move(rs)); + std::make_unique<CollectionImpl>(opCtx, ns, RecordId(0), UUID::gen(), std::move(rs)); uow.commit(); } @@ -672,9 +719,10 @@ DEATH_TEST_REGEX_F(DurableCatalogImplTest, << "key" << BSON("a" << 1))); std::unique_ptr<SortedDataInterface> sorted; { - MyOperationContext opCtx(engine); - ASSERT_OK(engine->createSortedDataInterface(&opCtx, CollectionOptions(), ident, &desc)); - sorted = engine->getSortedDataInterface(&opCtx, ident, &desc); + auto clientAndCtx = makeClientAndCtx("opCtx"); + auto opCtx = clientAndCtx.opCtx(); + ASSERT_OK(engine->createSortedDataInterface(opCtx, CollectionOptions(), ident, &desc)); + sorted = engine->getSortedDataInterface(opCtx, ident, &desc); ASSERT(sorted); } } diff --git a/src/mongo/db/storage/storage_engine_test_fixture.h b/src/mongo/db/storage/storage_engine_test_fixture.h index d8d76456373..a6b5ed024cd 100644 --- a/src/mongo/db/storage/storage_engine_test_fixture.h +++ b/src/mongo/db/storage/storage_engine_test_fixture.h @@ -34,6 +34,7 @@ #include "mongo/db/catalog/collection_catalog.h" #include "mongo/db/catalog/collection_mock.h" #include "mongo/db/catalog_raii.h" +#include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/service_context_d_test_fixture.h" #include "mongo/db/storage/durable_catalog.h" #include "mongo/db/storage/kv/kv_engine.h" @@ -49,7 +50,11 @@ public: : ServiceContextMongoDTest("ephemeralForTest", repair), _storageEngine(getServiceContext()->getStorageEngine()) {} - StorageEngineTest() : StorageEngineTest(RepairAction::kNoRepair) {} + StorageEngineTest() : StorageEngineTest(RepairAction::kNoRepair) { + auto serviceCtx = getServiceContext(); + repl::ReplicationCoordinator::set( + serviceCtx, std::make_unique<repl::ReplicationCoordinatorMock>(serviceCtx)); + } StatusWith<DurableCatalog::Entry> createCollection(OperationContext* opCtx, NamespaceString ns) { |