diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2021-10-01 17:35:46 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-10-04 20:36:46 +0000 |
commit | e24c4546b90a5af883dc111c0a60c6424ac395d2 (patch) | |
tree | 2321c53b3331edc4593ffa97004c168d18e80ce4 | |
parent | 5a7aade7b5939c5946f84b337886f861a33bab1a (diff) | |
download | mongo-e24c4546b90a5af883dc111c0a60c6424ac395d2.tar.gz |
SERVER-60343 Single-phase index builds perform timestamped catalog writes on abort
-rw-r--r-- | src/mongo/db/auth/auth_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/multi_index_block.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/catalog/multi_index_block.h | 8 | ||||
-rw-r--r-- | src/mongo/db/fcv_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/free_mon/free_mon_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/op_observer.h | 7 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.cpp | 61 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.h | 1 | ||||
-rw-r--r-- | src/mongo/db/op_observer_noop.h | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer_registry.h | 8 | ||||
-rw-r--r-- | src/mongo/db/repl/primary_only_service_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/tenant_migration_donor_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/tenant_migration_recipient_op_observer.h | 2 | ||||
-rw-r--r-- | src/mongo/db/s/config_server_op_observer.h | 3 | ||||
-rw-r--r-- | src/mongo/db/s/resharding/resharding_op_observer.h | 3 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_op_observer.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_op_observer.h | 2 |
18 files changed, 108 insertions, 19 deletions
diff --git a/src/mongo/db/auth/auth_op_observer.h b/src/mongo/db/auth/auth_op_observer.h index 5948d210362..811c7fe211f 100644 --- a/src/mongo/db/auth/auth_op_observer.h +++ b/src/mongo/db/auth/auth_op_observer.h @@ -60,6 +60,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp index 137793a4c42..978ed37cf25 100644 --- a/src/mongo/db/catalog/multi_index_block.cpp +++ b/src/mongo/db/catalog/multi_index_block.cpp @@ -91,6 +91,14 @@ MultiIndexBlock::~MultiIndexBlock() { MultiIndexBlock::OnCleanUpFn MultiIndexBlock::kNoopOnCleanUpFn = []() {}; +MultiIndexBlock::OnCleanUpFn MultiIndexBlock::makeTimestampedOnCleanUpFn( + OperationContext* opCtx, const CollectionPtr& coll) { + return [opCtx, ns = coll->ns()]() -> Status { + opCtx->getServiceContext()->getOpObserver()->onAbortIndexBuildSinglePhase(opCtx, ns); + return Status::OK(); + }; +} + void MultiIndexBlock::abortIndexBuild(OperationContext* opCtx, CollectionWriter& collection, OnCleanUpFn onCleanUp) noexcept { diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h index 333ea1e03c4..2dd45df2a7b 100644 --- a/src/mongo/db/catalog/multi_index_block.h +++ b/src/mongo/db/catalog/multi_index_block.h @@ -281,6 +281,14 @@ public: static OnCleanUpFn kNoopOnCleanUpFn; /** + * Returns an onCleanUp function for clean up when this index build should be timestamped. When + * called on primaries, this generates a new optime, writes a no-op oplog entry, and timestamps + * the catalog write to remove the index entry. Does nothing on secondaries. + */ + static OnCleanUpFn makeTimestampedOnCleanUpFn(OperationContext* opCtx, + const CollectionPtr& coll); + + /** * 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. May * delete internal tables, but this is not transactional. Writes the resumable index build diff --git a/src/mongo/db/fcv_op_observer.h b/src/mongo/db/fcv_op_observer.h index 7de88e944b8..0490023ec05 100644 --- a/src/mongo/db/fcv_op_observer.h +++ b/src/mongo/db/fcv_op_observer.h @@ -84,6 +84,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/free_mon/free_mon_op_observer.h b/src/mongo/db/free_mon/free_mon_op_observer.h index b2e73af1f41..8c985f0f58e 100644 --- a/src/mongo/db/free_mon/free_mon_op_observer.h +++ b/src/mongo/db/free_mon/free_mon_op_observer.h @@ -60,6 +60,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 6c6815d3e25..09badb3b528 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -1590,8 +1590,10 @@ void IndexBuildsCoordinator::createIndex(OperationContext* opCtx, } ScopeGuard abortOnExit([&] { - _indexBuildsManager.abortIndexBuild( - opCtx, collection, buildUUID, MultiIndexBlock::kNoopOnCleanUpFn); + // A timestamped transaction is needed to perform a catalog write that removes the index + // entry when aborting the single-phase index build for tenant migrations only. + auto onCleanUpFn = MultiIndexBlock::makeTimestampedOnCleanUpFn(opCtx, collection.get()); + _indexBuildsManager.abortIndexBuild(opCtx, collection, buildUUID, onCleanUpFn); }); uassertStatusOK(_indexBuildsManager.startBuildingIndex(opCtx, collection.get(), buildUUID)); diff --git a/src/mongo/db/op_observer.h b/src/mongo/db/op_observer.h index 6f1777e88c3..604561e0407 100644 --- a/src/mongo/db/op_observer.h +++ b/src/mongo/db/op_observer.h @@ -106,6 +106,13 @@ public: virtual void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) = 0; + /** + * Generates a timestamp by writing a no-op oplog entry. This is only necessary for tenant + * migrations that are aborting single-phase index builds. + */ + virtual void onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) = 0; + virtual void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index d8d52d1bc4c..b10e47fca8d 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -304,6 +304,31 @@ void writeToChangeStreamPreImagesCollection(OperationContext* opCtx, !res.existing && !res.upsertedId.isEmpty()); } +bool shouldTimestampIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) { + // This function returns whether a timestamp for a catalog write when beginning an index build, + // or aborting an index build is necessary. There are four scenarios: + + // 1. A timestamp is already set -- replication application sets a timestamp ahead of time. + // This could include the phase of initial sync where it applies oplog entries. Also, + // primaries performing an index build via `applyOps` may have a wrapping commit timestamp. + if (!opCtx->recoveryUnit()->getCommitTimestamp().isNull()) + return false; + + // 2. If the node is initial syncing, we do not set a timestamp. + auto replCoord = repl::ReplicationCoordinator::get(opCtx); + if (replCoord->isReplEnabled() && replCoord->getMemberState().startup2()) + return false; + + // 3. If the index build is on the local database, do not timestamp. + if (nss.isLocal()) + return false; + + // 4. All other cases, we generate a timestamp by writing a no-op oplog entry. This is + // better than using a ghost timestamp. Writing an oplog entry ensures this node is + // primary. + return true; +} + } // namespace BSONObj OpObserverImpl::DocumentKey::getId() const { @@ -397,32 +422,34 @@ void OpObserverImpl::onStartIndexBuild(OperationContext* opCtx, void OpObserverImpl::onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) { - // This function sets a timestamp for the initial catalog write when beginning an index - // build, if necessary. There are four scenarios: - - // 1. A timestamp is already set -- replication application sets a timestamp ahead of time. - // This could include the phase of initial sync where it applies oplog entries. Also, - // primaries performing an index build via `applyOps` may have a wrapping commit timestamp. - if (!opCtx->recoveryUnit()->getCommitTimestamp().isNull()) + if (!shouldTimestampIndexBuildSinglePhase(opCtx, nss)) { return; + } - // 2. If the node is initial syncing, we do not set a timestamp. - auto replCoord = repl::ReplicationCoordinator::get(opCtx); - if (replCoord->isReplEnabled() && replCoord->getMemberState().startup2()) - return; - // 3. If the index build is on the local database, do not timestamp. - if (nss.isLocal()) + onInternalOpMessage( + opCtx, + {}, + boost::none, + BSON("msg" << std::string(str::stream() << "Creating indexes. Coll: " << nss)), + boost::none, + boost::none, + boost::none, + boost::none, + boost::none); +} + +void OpObserverImpl::onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) { + if (!shouldTimestampIndexBuildSinglePhase(opCtx, nss)) { return; + } - // 4. All other cases, we generate a timestamp by writing a no-op oplog entry. This is - // better than using a ghost timestamp. Writing an oplog entry ensures this node is - // primary. onInternalOpMessage( opCtx, {}, boost::none, - BSON("msg" << std::string(str::stream() << "Creating indexes. Coll: " << nss)), + BSON("msg" << std::string(str::stream() << "Aborting indexes. Coll: " << nss)), boost::none, boost::none, boost::none, diff --git a/src/mongo/db/op_observer_impl.h b/src/mongo/db/op_observer_impl.h index 2fe3637eeb5..dc3f948d640 100644 --- a/src/mongo/db/op_observer_impl.h +++ b/src/mongo/db/op_observer_impl.h @@ -78,6 +78,7 @@ public: const std::vector<BSONObj>& indexes, bool fromMigrate) final; void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final; + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final; void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, diff --git a/src/mongo/db/op_observer_noop.h b/src/mongo/db/op_observer_noop.h index 70f4589eafe..8c859c7a03b 100644 --- a/src/mongo/db/op_observer_noop.h +++ b/src/mongo/db/op_observer_noop.h @@ -51,6 +51,9 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) override {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) override {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/op_observer_registry.h b/src/mongo/db/op_observer_registry.h index 2b6b0db7e81..afb039743c4 100644 --- a/src/mongo/db/op_observer_registry.h +++ b/src/mongo/db/op_observer_registry.h @@ -88,6 +88,14 @@ public: } } + virtual void onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) override { + ReservedTimes times{opCtx}; + for (auto& o : _observers) { + o->onAbortIndexBuildSinglePhase(opCtx, nss); + } + } + virtual void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/repl/primary_only_service_op_observer.h b/src/mongo/db/repl/primary_only_service_op_observer.h index 2dd563137a4..dec4118f887 100644 --- a/src/mongo/db/repl/primary_only_service_op_observer.h +++ b/src/mongo/db/repl/primary_only_service_op_observer.h @@ -62,6 +62,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/repl/tenant_migration_donor_op_observer.h b/src/mongo/db/repl/tenant_migration_donor_op_observer.h index 3424ec154d6..da948095750 100644 --- a/src/mongo/db/repl/tenant_migration_donor_op_observer.h +++ b/src/mongo/db/repl/tenant_migration_donor_op_observer.h @@ -60,6 +60,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/repl/tenant_migration_recipient_op_observer.h b/src/mongo/db/repl/tenant_migration_recipient_op_observer.h index a4d3092037b..ae654e3c6a1 100644 --- a/src/mongo/db/repl/tenant_migration_recipient_op_observer.h +++ b/src/mongo/db/repl/tenant_migration_recipient_op_observer.h @@ -61,6 +61,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) final {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/s/config_server_op_observer.h b/src/mongo/db/s/config_server_op_observer.h index f7d2f65029c..c5bee193153 100644 --- a/src/mongo/db/s/config_server_op_observer.h +++ b/src/mongo/db/s/config_server_op_observer.h @@ -62,6 +62,9 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) override {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) override {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/s/resharding/resharding_op_observer.h b/src/mongo/db/s/resharding/resharding_op_observer.h index 5ddcd31d8d4..8d11ea97b5b 100644 --- a/src/mongo/db/s/resharding/resharding_op_observer.h +++ b/src/mongo/db/s/resharding/resharding_op_observer.h @@ -77,6 +77,9 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) override {} + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) override {} + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, diff --git a/src/mongo/db/s/shard_server_op_observer.cpp b/src/mongo/db/s/shard_server_op_observer.cpp index f78889954f3..ca64c7df032 100644 --- a/src/mongo/db/s/shard_server_op_observer.cpp +++ b/src/mongo/db/s/shard_server_op_observer.cpp @@ -619,6 +619,11 @@ void ShardServerOpObserver::onStartIndexBuildSinglePhase(OperationContext* opCtx abortOngoingMigrationIfNeeded(opCtx, nss); } +void ShardServerOpObserver::onAbortIndexBuildSinglePhase(OperationContext* opCtx, + const NamespaceString& nss) { + abortOngoingMigrationIfNeeded(opCtx, nss); +} + void ShardServerOpObserver::onDropIndex(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, diff --git a/src/mongo/db/s/shard_server_op_observer.h b/src/mongo/db/s/shard_server_op_observer.h index eff1924ffd6..7bc352bbdfe 100644 --- a/src/mongo/db/s/shard_server_op_observer.h +++ b/src/mongo/db/s/shard_server_op_observer.h @@ -60,6 +60,8 @@ public: void onStartIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) override; + void onAbortIndexBuildSinglePhase(OperationContext* opCtx, const NamespaceString& nss) override; + void onCommitIndexBuild(OperationContext* opCtx, const NamespaceString& nss, CollectionUUID collUUID, |