diff options
author | Maria van Keulen <maria@mongodb.com> | 2018-05-17 15:12:05 -0400 |
---|---|---|
committer | Maria van Keulen <maria@mongodb.com> | 2018-06-06 16:54:44 -0400 |
commit | 7c12245583958023e35bce05d6b2212dcd7f24e3 (patch) | |
tree | 4cfa8bb4915cef1973b208d33fa6023b1c5cdb65 /src | |
parent | 0b001ed1ab7e8443213db38030cf17d2a6f5be2c (diff) | |
download | mongo-7c12245583958023e35bce05d6b2212dcd7f24e3.tar.gz |
SERVER-34615 Make UUIDCatalog updates for renameCollection atomic
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/catalog/rename_collection.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/catalog/rename_collection_test.cpp | 72 | ||||
-rw-r--r-- | src/mongo/db/catalog/uuid_catalog.cpp | 88 | ||||
-rw-r--r-- | src/mongo/db/catalog/uuid_catalog.h | 36 | ||||
-rw-r--r-- | src/mongo/db/catalog/uuid_catalog_test.cpp | 13 | ||||
-rw-r--r-- | src/mongo/db/free_mon/free_mon_op_observer.h | 26 | ||||
-rw-r--r-- | src/mongo/db/op_observer.h | 40 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.cpp | 44 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.h | 24 | ||||
-rw-r--r-- | src/mongo/db/op_observer_noop.h | 24 | ||||
-rw-r--r-- | src/mongo/db/op_observer_registry.h | 39 | ||||
-rw-r--r-- | src/mongo/db/op_observer_registry_test.cpp | 39 | ||||
-rw-r--r-- | src/mongo/db/repl/storage_interface_impl.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/s/config_server_op_observer.h | 24 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_op_observer.h | 24 |
15 files changed, 372 insertions, 128 deletions
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index c41ea778782..305750104e7 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -258,7 +258,7 @@ Status renameCollectionCommon(OperationContext* opCtx, invariant(options.dropTarget); auto dropTargetUUID = targetColl->uuid(); invariant(dropTargetUUID); - auto renameOpTime = opObserver->onRenameCollection( + auto renameOpTime = opObserver->preRenameCollection( opCtx, source, target, sourceUUID, dropTargetUUID, options.stayTemp); if (!renameOpTimeFromApplyOps.isNull()) { @@ -286,6 +286,8 @@ Status renameCollectionCommon(OperationContext* opCtx, return status; } + opObserver->postRenameCollection( + opCtx, source, target, sourceUUID, dropTargetUUID, options.stayTemp); wunit.commit(); return Status::OK(); }); diff --git a/src/mongo/db/catalog/rename_collection_test.cpp b/src/mongo/db/catalog/rename_collection_test.cpp index ede724e4968..52bfaf4c8e8 100644 --- a/src/mongo/db/catalog/rename_collection_test.cpp +++ b/src/mongo/db/catalog/rename_collection_test.cpp @@ -94,13 +94,25 @@ public: const NamespaceString& collectionName, OptionalCollectionUUID uuid) override; - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override; - + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; // Operations written to the oplog. These are operations for which // ReplicationCoordinator::isOplogDisabled() returns false. std::vector<std::string> oplogEntries; @@ -167,21 +179,43 @@ repl::OpTime OpObserverMock::onDropCollection(OperationContext* opCtx, return {}; } -repl::OpTime OpObserverMock::onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) { - _logOp(opCtx, fromCollection, "rename"); - OpObserver::Times::get(opCtx).reservedOpTimes.push_back(renameOpTime); +void OpObserverMock::onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + preRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); OpObserverNoop::onRenameCollection( opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); onRenameCollectionCalled = true; onRenameCollectionDropTarget = dropTargetUUID; - return {}; } +void OpObserverMock::postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + OpObserverNoop::postRenameCollection( + opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + onRenameCollectionCalled = true; + onRenameCollectionDropTarget = dropTargetUUID; +} + +repl::OpTime OpObserverMock::preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + _logOp(opCtx, fromCollection, "rename"); + OpObserver::Times::get(opCtx).reservedOpTimes.push_back(renameOpTime); + OpObserverNoop::preRenameCollection( + opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + return {}; +} void OpObserverMock::_logOp(OperationContext* opCtx, const NamespaceString& nss, const std::string& operationName) { @@ -566,6 +600,10 @@ TEST_F(RenameCollectionTest, RenameCollectionForApplyOpsDropTargetByUUIDTargetDo ASSERT_FALSE(_collectionExists(_opCtx.get(), collC)); // B (originally A) should exist ASSERT_TRUE(_collectionExists(_opCtx.get(), collB)); + // collAUUID should be associated with collB's NamespaceString in the UUIDCatalog. + auto newCollNS = _getCollectionNssFromUUID(_opCtx.get(), collAUUID); + ASSERT_TRUE(newCollNS.isValid()); + ASSERT_EQUALS(newCollNS, collB); } TEST_F(RenameCollectionTest, RenameCollectionForApplyOpsDropTargetByUUIDTargetExists) { @@ -718,7 +756,7 @@ TEST_F(RenameCollectionTest, repl::UnreplicatedWritesBlock uwb(_opCtx.get()); ASSERT_FALSE(_opCtx->writesAreReplicated()); - // OpObserver::onRenameCollection() must return a null OpTime when writes are not replicated. + // OpObserver::preRenameCollection() must return a null OpTime when writes are not replicated. _opObserver->renameOpTime = {}; _createCollection(_opCtx.get(), _sourceNss); diff --git a/src/mongo/db/catalog/uuid_catalog.cpp b/src/mongo/db/catalog/uuid_catalog.cpp index 7b43be4f1b1..91595b5c69b 100644 --- a/src/mongo/db/catalog/uuid_catalog.cpp +++ b/src/mongo/db/catalog/uuid_catalog.cpp @@ -82,26 +82,42 @@ repl::OpTime UUIDCatalogObserver::onDropCollection(OperationContext* opCtx, return {}; } -repl::OpTime UUIDCatalogObserver::onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) { +void UUIDCatalogObserver::onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { if (!uuid) - return {}; - auto getNewCollection = [opCtx, toCollection] { - auto db = DatabaseHolder::getDatabaseHolder().get(opCtx, toCollection.db()); - auto newColl = db->getCollection(opCtx, toCollection); - invariant(newColl); - return newColl; - }; + return; + auto db = DatabaseHolder::getDatabaseHolder().get(opCtx, toCollection.db()); + auto newColl = db->getCollection(opCtx, toCollection); + invariant(newColl); UUIDCatalog& catalog = UUIDCatalog::get(opCtx); - catalog.onRenameCollection(opCtx, getNewCollection, uuid.get()); + catalog.onRenameCollection(opCtx, newColl, uuid.get()); +} + +repl::OpTime UUIDCatalogObserver::preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { return {}; } +void UUIDCatalogObserver::postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + // postRenameCollection and onRenameCollection are semantically equivalent from the perspective + // of the UUIDCatalogObserver. + onRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); +} + UUIDCatalog& UUIDCatalog::get(ServiceContext* svcCtx) { return getCatalog(svcCtx); } @@ -124,25 +140,12 @@ void UUIDCatalog::onDropCollection(OperationContext* opCtx, CollectionUUID uuid) } void UUIDCatalog::onRenameCollection(OperationContext* opCtx, - GetNewCollectionFunction getNewCollection, + Collection* coll, CollectionUUID uuid) { - Collection* oldColl = removeUUIDCatalogEntry(uuid); - opCtx->recoveryUnit()->onCommit([this, getNewCollection, uuid](boost::optional<Timestamp>) { - // Reset current UUID entry in case some other operation updates the UUID catalog before the - // WUOW is committed. registerUUIDCatalogEntry() is a no-op if there's an existing UUID - // entry. - removeUUIDCatalogEntry(uuid); - auto newColl = getNewCollection(); - invariant(newColl); - registerUUIDCatalogEntry(uuid, newColl); - }); - opCtx->recoveryUnit()->onRollback([this, oldColl, uuid] { - // Reset current UUID entry in case some other operation updates the UUID catalog before the - // WUOW is rolled back. registerUUIDCatalogEntry() is a no-op if there's an existing UUID - // entry. - removeUUIDCatalogEntry(uuid); - registerUUIDCatalogEntry(uuid, oldColl); - }); + invariant(coll); + Collection* oldColl = replaceUUIDCatalogEntry(uuid, coll); + opCtx->recoveryUnit()->onRollback( + [this, oldColl, uuid] { replaceUUIDCatalogEntry(uuid, oldColl); }); } void UUIDCatalog::onCloseDatabase(Database* db) { @@ -192,6 +195,27 @@ NamespaceString UUIDCatalog::lookupNSSByUUID(CollectionUUID uuid) const { return NamespaceString(); } +Collection* UUIDCatalog::replaceUUIDCatalogEntry(CollectionUUID uuid, Collection* coll) { + stdx::lock_guard<stdx::mutex> lock(_catalogLock); + invariant(coll); + + auto foundIt = _catalog.find(uuid); + invariant(foundIt != _catalog.end()); + // Invalidate the source database's ordering, since we're deleting a UUID. + _orderedCollections.erase(foundIt->second->ns().db()); + + Collection* oldColl = foundIt->second; + LOG(2) << "unregistering collection " << oldColl->ns() << " with UUID " << uuid.toString(); + _catalog.erase(foundIt); + + // Invalidate the destination database's ordering, since we're adding a new UUID. + _orderedCollections.erase(coll->ns().db()); + + std::pair<CollectionUUID, Collection*> entry = std::make_pair(uuid, coll); + LOG(2) << "registering collection " << coll->ns() << " with UUID " << uuid.toString(); + invariant(_catalog.insert(entry).second == true); + return oldColl; +} void UUIDCatalog::registerUUIDCatalogEntry(CollectionUUID uuid, Collection* coll) { stdx::lock_guard<stdx::mutex> lock(_catalogLock); diff --git a/src/mongo/db/catalog/uuid_catalog.h b/src/mongo/db/catalog/uuid_catalog.h index aa864eb2571..a96210856fc 100644 --- a/src/mongo/db/catalog/uuid_catalog.h +++ b/src/mongo/db/catalog/uuid_catalog.h @@ -89,12 +89,24 @@ public: OptionalCollectionUUID uuid, const std::string& indexName, const BSONObj& idxDescriptor) override {} - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override; + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; void onApplyOps(OperationContext* opCtx, const std::string& dbName, const BSONObj& applyOpCmd) override {} @@ -136,20 +148,18 @@ public: void onDropCollection(OperationContext* opCtx, CollectionUUID uuid); /** - * Combination of onDropCollection and onCreateCollection. - * 'getNewCollection' is a function that returns collection to be registered when the current - * write unit of work is committed. + * This function atomically removes any existing entry for uuid from the UUID catalog and adds + * a new entry for uuid associated with the Collection coll. It is called by the op observer + * when a collection is renamed. */ - using GetNewCollectionFunction = stdx::function<Collection*()>; - void onRenameCollection(OperationContext* opCtx, - GetNewCollectionFunction getNewCollection, - CollectionUUID uuid); + void onRenameCollection(OperationContext* opCtx, Collection* coll, CollectionUUID uuid); /** * Implies onDropCollection for all collections in db, but is not transactional. */ void onCloseDatabase(Database* db); + Collection* replaceUUIDCatalogEntry(CollectionUUID uuid, Collection* coll); void registerUUIDCatalogEntry(CollectionUUID uuid, Collection* coll); Collection* removeUUIDCatalogEntry(CollectionUUID uuid); diff --git a/src/mongo/db/catalog/uuid_catalog_test.cpp b/src/mongo/db/catalog/uuid_catalog_test.cpp index 0028f7d38f4..097aea9c029 100644 --- a/src/mongo/db/catalog/uuid_catalog_test.cpp +++ b/src/mongo/db/catalog/uuid_catalog_test.cpp @@ -110,6 +110,19 @@ TEST_F(UUIDCatalogTest, OnDropCollection) { ASSERT(catalog.lookupCollectionByUUID(colUUID) == nullptr); } +TEST_F(UUIDCatalogTest, OnRenameCollection) { + auto oldUUID = CollectionUUID::gen(); + NamespaceString oldNss(nss.db(), "oldcol"); + Collection oldCol(stdx::make_unique<CollectionMock>(oldNss)); + catalog.onCreateCollection(&opCtx, &oldCol, oldUUID); + ASSERT_EQUALS(catalog.lookupCollectionByUUID(oldUUID), &oldCol); + + NamespaceString newNss(nss.db(), "newcol"); + Collection newCol(stdx::make_unique<CollectionMock>(newNss)); + catalog.onRenameCollection(&opCtx, &newCol, oldUUID); + ASSERT_EQUALS(catalog.lookupCollectionByUUID(oldUUID), &newCol); +} + TEST_F(UUIDCatalogTest, NonExistingNextCol) { ASSERT_FALSE(catalog.next(nss.db(), colUUID)); ASSERT_FALSE(catalog.next(nss.db(), nextUUID)); 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 acdb03d4890..c084c440dd2 100644 --- a/src/mongo/db/free_mon/free_mon_op_observer.h +++ b/src/mongo/db/free_mon/free_mon_op_observer.h @@ -101,15 +101,27 @@ public: const std::string& indexName, const BSONObj& indexInfo) final {} - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) final { + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) final {} + + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) final { return repl::OpTime(); } - + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) final {} void onApplyOps(OperationContext* opCtx, const std::string& dbName, const BSONObj& applyOpCmd) final {} diff --git a/src/mongo/db/op_observer.h b/src/mongo/db/op_observer.h index d65bb57bb7b..9a543aa0aa4 100644 --- a/src/mongo/db/op_observer.h +++ b/src/mongo/db/op_observer.h @@ -214,16 +214,42 @@ public: /** * This function logs an oplog entry when a 'renameCollection' command on a collection is - * executed. + * executed. It should be used specifically in instances where the optime is necessary to + * be obtained prior to performing the actual rename, and should only be used in conjunction + * with postRenameCollection. * Returns the optime of the oplog entry successfully written to the oplog. * Returns a null optime if an oplog entry was not written for this operation. */ - virtual repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) = 0; + virtual repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) = 0; + /** + * This function performs all op observer handling for a 'renameCollection' command except for + * logging the oplog entry. It should be used specifically in instances where the optime is + * necessary to be obtained prior to performing the actual rename, and should only be used in + * conjunction with preRenameCollection. + */ + virtual void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) = 0; + /** + * This function logs an oplog entry when a 'renameCollection' command on a collection is + * executed. It calls preRenameCollection to log the entry and postRenameCollection to do all + * other handling. + */ + virtual void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) = 0; + virtual void onApplyOps(OperationContext* opCtx, const std::string& dbName, const BSONObj& applyOpCmd) = 0; diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index 40173774735..aa85d823222 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -766,12 +766,13 @@ void OpObserverImpl::onDropIndex(OperationContext* opCtx, ->logOp(opCtx, "c", cmdNss, cmdObj, &indexInfo); } -repl::OpTime OpObserverImpl::onRenameCollection(OperationContext* const opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) { + +repl::OpTime OpObserverImpl::preRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { const auto cmdNss = fromCollection.getCommandNS(); BSONObjBuilder builder; @@ -796,6 +797,27 @@ repl::OpTime OpObserverImpl::onRenameCollection(OperationContext* const opCtx, kUninitializedStmtId, {}); + return {}; +} + +void OpObserverImpl::postRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + const auto cmdNss = fromCollection.getCommandNS(); + + BSONObjBuilder builder; + builder.append("renameCollection", fromCollection.ns()); + builder.append("to", toCollection.ns()); + builder.append("stayTemp", stayTemp); + if (dropTargetUUID) { + dropTargetUUID->appendToBuilder(&builder, "dropTarget"); + } + + const auto cmdObj = builder.done(); + if (fromCollection.isSystemDotViews()) DurableViewCatalog::onExternalChange(opCtx, fromCollection); if (toCollection.isSystemDotViews()) @@ -810,8 +832,16 @@ repl::OpTime OpObserverImpl::onRenameCollection(OperationContext* const opCtx, cache.evictNamespace(toCollection); opCtx->recoveryUnit()->onRollback( [&cache, toCollection]() { cache.evictNamespace(toCollection); }); +} - return {}; +void OpObserverImpl::onRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + preRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + postRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); } void OpObserverImpl::onApplyOps(OperationContext* opCtx, diff --git a/src/mongo/db/op_observer_impl.h b/src/mongo/db/op_observer_impl.h index 4d62e4d8b3a..fb67efbe104 100644 --- a/src/mongo/db/op_observer_impl.h +++ b/src/mongo/db/op_observer_impl.h @@ -85,12 +85,24 @@ public: OptionalCollectionUUID uuid, const std::string& indexName, const BSONObj& indexInfo) override; - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override; + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override; void onApplyOps(OperationContext* opCtx, const std::string& dbName, const BSONObj& applyOpCmd) override; diff --git a/src/mongo/db/op_observer_noop.h b/src/mongo/db/op_observer_noop.h index 67682888d1a..5d74fa914af 100644 --- a/src/mongo/db/op_observer_noop.h +++ b/src/mongo/db/op_observer_noop.h @@ -82,14 +82,26 @@ public: OptionalCollectionUUID uuid, const std::string& indexName, const BSONObj& idxDescriptor) override {} - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override { + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { return {}; } + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} void onApplyOps(OperationContext* opCtx, const std::string& dbName, const BSONObj& applyOpCmd) override {} diff --git a/src/mongo/db/op_observer_registry.h b/src/mongo/db/op_observer_registry.h index d02025ffd98..2ab2b971219 100644 --- a/src/mongo/db/op_observer_registry.h +++ b/src/mongo/db/op_observer_registry.h @@ -160,22 +160,45 @@ public: o->onDropIndex(opCtx, nss, uuid, indexName, idxDescriptor); } - repl::OpTime onRenameCollection(OperationContext* const opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override { + + void onRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { + ReservedTimes times{opCtx}; + for (auto& o : _observers) + o->onRenameCollection( + opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + } + + repl::OpTime preRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { ReservedTimes times{opCtx}; for (auto& observer : this->_observers) { - const auto time = observer->onRenameCollection( + const auto time = observer->preRenameCollection( opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); invariant(time.isNull()); } - return _getOpTimeToReturn(times.get().reservedOpTimes); } + void postRenameCollection(OperationContext* const opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { + ReservedTimes times{opCtx}; + for (auto& o : _observers) + o->postRenameCollection( + opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + } void onApplyOps(OperationContext* const opCtx, const std::string& dbName, const BSONObj& applyOpCmd) override { diff --git a/src/mongo/db/op_observer_registry_test.cpp b/src/mongo/db/op_observer_registry_test.cpp index 445c4789aab..f1873b25c94 100644 --- a/src/mongo/db/op_observer_registry_test.cpp +++ b/src/mongo/db/op_observer_registry_test.cpp @@ -59,15 +59,30 @@ struct TestObserver : public OpObserverNoop { OpObserver::Times::get(opCtx).reservedOpTimes.push_back(opTime); return {}; } - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) { + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { + preRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + postRenameCollection(opCtx, fromCollection, toCollection, uuid, dropTargetUUID, stayTemp); + } + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) { OpObserver::Times::get(opCtx).reservedOpTimes.push_back(opTime); return {}; } + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) {} }; struct ThrowingObserver : public TestObserver { @@ -156,12 +171,14 @@ TEST_F(OpObserverRegistryTest, OnDropCollectionObserverResultReturnsRightTime) { checkConsistentOpTime(op); } -TEST_F(OpObserverRegistryTest, OnRenameCollectionObserverResultReturnsRightTime) { +TEST_F(OpObserverRegistryTest, PreRenameCollectionObserverResultReturnsRightTime) { OperationContextNoop opCtx; registry.addObserver(std::move(unique1)); registry.addObserver(std::make_unique<OpObserverNoop>()); auto op = [&]() -> repl::OpTime { - return registry.onRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + auto opTime = registry.preRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + registry.postRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + return opTime; }; checkConsistentOpTime(op); } @@ -174,12 +191,14 @@ DEATH_TEST_F(OpObserverRegistryTest, OnDropCollectionReturnsInconsistentTime, "i checkInconsistentOpTime(op); } -DEATH_TEST_F(OpObserverRegistryTest, OnRenameCollectionReturnsInconsistentTime, "invariant") { +DEATH_TEST_F(OpObserverRegistryTest, PreRenameCollectionReturnsInconsistentTime, "invariant") { OperationContextNoop opCtx; registry.addObserver(std::move(unique1)); registry.addObserver(std::move(unique2)); auto op = [&]() -> repl::OpTime { - return registry.onRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + auto opTime = registry.preRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + registry.postRenameCollection(&opCtx, testNss, testNss, {}, {}, false); + return opTime; }; checkInconsistentOpTime(op); } diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index c7dc39c8b68..d271adf849d 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -491,8 +491,7 @@ Status StorageInterfaceImpl::renameCollection(OperationContext* opCtx, auto newColl = autoDB.getDb()->getCollection(opCtx, toNS); if (newColl->uuid()) { - UUIDCatalog::get(opCtx).onRenameCollection( - opCtx, [newColl] { return newColl; }, newColl->uuid().get()); + UUIDCatalog::get(opCtx).onRenameCollection(opCtx, newColl, newColl->uuid().get()); } wunit.commit(); return status; diff --git a/src/mongo/db/s/config_server_op_observer.h b/src/mongo/db/s/config_server_op_observer.h index d2e60094f7d..91870bf4732 100644 --- a/src/mongo/db/s/config_server_op_observer.h +++ b/src/mongo/db/s/config_server_op_observer.h @@ -101,14 +101,26 @@ public: const std::string& indexName, const BSONObj& indexInfo) override {} - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override { + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { return repl::OpTime(); } + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} void onApplyOps(OperationContext* opCtx, const std::string& dbName, diff --git a/src/mongo/db/s/shard_server_op_observer.h b/src/mongo/db/s/shard_server_op_observer.h index 5f5e6723516..413373446bd 100644 --- a/src/mongo/db/s/shard_server_op_observer.h +++ b/src/mongo/db/s/shard_server_op_observer.h @@ -102,14 +102,26 @@ public: const std::string& indexName, const BSONObj& indexInfo) override {} - repl::OpTime onRenameCollection(OperationContext* opCtx, - const NamespaceString& fromCollection, - const NamespaceString& toCollection, - OptionalCollectionUUID uuid, - OptionalCollectionUUID dropTargetUUID, - bool stayTemp) override { + void onRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} + repl::OpTime preRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override { return repl::OpTime(); } + void postRenameCollection(OperationContext* opCtx, + const NamespaceString& fromCollection, + const NamespaceString& toCollection, + OptionalCollectionUUID uuid, + OptionalCollectionUUID dropTargetUUID, + bool stayTemp) override {} void onApplyOps(OperationContext* opCtx, const std::string& dbName, |