From 961107ba6513af4db1e5eed6b57f24a735aaff61 Mon Sep 17 00:00:00 2001 From: Enrico Golfieri Date: Thu, 9 Feb 2023 09:25:27 +0000 Subject: SERVER-72235 Core Server Avoid the usage of dropCollectionEvenIfSystem() outside of resharding code --- .../db/s/resharding/resharding_data_copy_util.cpp | 27 ++++++++++++++++-- .../db/s/resharding/resharding_data_copy_util.h | 11 ++++++++ .../db/s/resharding/resharding_donor_service.cpp | 3 +- .../s/resharding/resharding_donor_service_test.cpp | 7 ++--- .../s/resharding/resharding_recipient_service.cpp | 2 +- .../resharding_recipient_service_test.cpp | 7 ++--- src/mongo/db/s/sharding_ddl_util.cpp | 32 ---------------------- src/mongo/db/s/sharding_ddl_util.h | 10 ------- 8 files changed, 41 insertions(+), 58 deletions(-) diff --git a/src/mongo/db/s/resharding/resharding_data_copy_util.cpp b/src/mongo/db/s/resharding/resharding_data_copy_util.cpp index 80b2f190e1e..d5fbd3d2bc1 100644 --- a/src/mongo/db/s/resharding/resharding_data_copy_util.cpp +++ b/src/mongo/db/s/resharding/resharding_data_copy_util.cpp @@ -45,7 +45,6 @@ #include "mongo/db/s/resharding/resharding_txn_cloner_progress_gen.h" #include "mongo/db/s/resharding/resharding_util.h" #include "mongo/db/s/session_catalog_migration.h" -#include "mongo/db/s/sharding_ddl_util.h" #include "mongo/db/session/session_catalog_mongod.h" #include "mongo/db/session/session_txn_record_gen.h" #include "mongo/db/storage/write_unit_of_work.h" @@ -73,6 +72,28 @@ void ensureCollectionExists(OperationContext* opCtx, }); } +void ensureCollectionDropped(OperationContext* opCtx, + const NamespaceString& nss, + const boost::optional& uuid) { + invariant(!opCtx->lockState()->isLocked()); + invariant(!opCtx->lockState()->inAWriteUnitOfWork()); + + writeConflictRetry( + opCtx, "resharding::data_copy::ensureCollectionDropped", nss.toString(), [&] { + AutoGetCollection coll(opCtx, nss, MODE_X); + if (!coll || (uuid && coll->uuid() != uuid)) { + // If the collection doesn't exist or exists with a different UUID, then the + // requested collection has been dropped already. + return; + } + + WriteUnitOfWork wuow(opCtx); + uassertStatusOK(coll.getDb()->dropCollectionEvenIfSystem( + opCtx, nss, {} /* dropOpTime */, true /* markFromMigrate */)); + wuow.commit(); + }); +} + void ensureOplogCollectionsDropped(OperationContext* opCtx, const UUID& reshardingUUID, const UUID& sourceUUID, @@ -99,11 +120,11 @@ void ensureOplogCollectionsDropped(OperationContext* opCtx, // Drop the conflict stash collection for this donor. auto stashNss = getLocalConflictStashNamespace(sourceUUID, donor.getShardId()); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent(opCtx, stashNss); + resharding::data_copy::ensureCollectionDropped(opCtx, stashNss); // Drop the oplog buffer collection for this donor. auto oplogBufferNss = getLocalOplogBufferNamespace(sourceUUID, donor.getShardId()); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent(opCtx, oplogBufferNss); + resharding::data_copy::ensureCollectionDropped(opCtx, oplogBufferNss); } } diff --git a/src/mongo/db/s/resharding/resharding_data_copy_util.h b/src/mongo/db/s/resharding/resharding_data_copy_util.h index bf3f21a7118..79dfb3ef36e 100644 --- a/src/mongo/db/s/resharding/resharding_data_copy_util.h +++ b/src/mongo/db/s/resharding/resharding_data_copy_util.h @@ -59,6 +59,17 @@ void ensureCollectionExists(OperationContext* opCtx, const NamespaceString& nss, const CollectionOptions& options); +/** + * Drops the specified collection or returns without error if the collection has already been + * dropped. A particular incarnation of the collection can be dropped by specifying its UUID. + * + * This functions assumes the collection being dropped doesn't have any two-phase index builds + * active on it. + */ +void ensureCollectionDropped(OperationContext* opCtx, + const NamespaceString& nss, + const boost::optional& uuid = boost::none); + /** * Removes documents from the oplog applier progress and transaction applier progress collections * that are associated with an in-progress resharding operation. Also drops all oplog buffer diff --git a/src/mongo/db/s/resharding/resharding_donor_service.cpp b/src/mongo/db/s/resharding/resharding_donor_service.cpp index e59af78881c..5f283410cb5 100644 --- a/src/mongo/db/s/resharding/resharding_donor_service.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_service.cpp @@ -53,7 +53,6 @@ #include "mongo/db/s/resharding/resharding_future_util.h" #include "mongo/db/s/resharding/resharding_server_parameters_gen.h" #include "mongo/db/s/resharding/resharding_util.h" -#include "mongo/db/s/sharding_ddl_util.h" #include "mongo/db/s/sharding_recovery_service.h" #include "mongo/db/s/sharding_state.h" #include "mongo/db/write_block_bypass.h" @@ -831,7 +830,7 @@ void ReshardingDonorService::DonorStateMachine::_dropOriginalCollectionThenTrans serverGlobalParams.featureCompatibility)) { dropCollectionGlobalIndexesMetadata(opCtx.get(), _metadata.getSourceNss()); } - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent( + resharding::data_copy::ensureCollectionDropped( opCtx.get(), _metadata.getSourceNss(), _metadata.getSourceUUID()); } diff --git a/src/mongo/db/s/resharding/resharding_donor_service_test.cpp b/src/mongo/db/s/resharding/resharding_donor_service_test.cpp index f2ea3c60e4b..53c0044ae75 100644 --- a/src/mongo/db/s/resharding/resharding_donor_service_test.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_service_test.cpp @@ -49,7 +49,6 @@ #include "mongo/db/s/resharding/resharding_donor_service.h" #include "mongo/db/s/resharding/resharding_service_test_helpers.h" #include "mongo/db/s/resharding/resharding_util.h" -#include "mongo/db/s/sharding_ddl_util.h" #include "mongo/db/s/sharding_recovery_service.h" #include "mongo/db/service_context.h" #include "mongo/logv2/log.h" @@ -166,8 +165,7 @@ public: void createSourceCollection(OperationContext* opCtx, const ReshardingDonorDocument& donorDoc) { CollectionOptions options; options.uuid = donorDoc.getSourceUUID(); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent(opCtx, - donorDoc.getSourceNss()); + resharding::data_copy::ensureCollectionDropped(opCtx, donorDoc.getSourceNss()); resharding::data_copy::ensureCollectionExists(opCtx, donorDoc.getSourceNss(), options); } @@ -175,8 +173,7 @@ public: const ReshardingDonorDocument& donorDoc) { CollectionOptions options; options.uuid = donorDoc.getReshardingUUID(); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent( - opCtx, donorDoc.getTempReshardingNss()); + resharding::data_copy::ensureCollectionDropped(opCtx, donorDoc.getTempReshardingNss()); resharding::data_copy::ensureCollectionExists( opCtx, donorDoc.getTempReshardingNss(), options); } diff --git a/src/mongo/db/s/resharding/resharding_recipient_service.cpp b/src/mongo/db/s/resharding/resharding_recipient_service.cpp index ff5a849ca78..062814fdc53 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service.cpp @@ -859,7 +859,7 @@ void ReshardingRecipientService::RecipientStateMachine::_cleanupReshardingCollec dropCollectionGlobalIndexesMetadata(opCtx.get(), _metadata.getTempReshardingNss()); } - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent( + resharding::data_copy::ensureCollectionDropped( opCtx.get(), _metadata.getTempReshardingNss(), _metadata.getReshardingUUID()); } } diff --git a/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp b/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp index 8664473c978..59a76e3da98 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp @@ -47,7 +47,6 @@ #include "mongo/db/s/resharding/resharding_recipient_service.h" #include "mongo/db/s/resharding/resharding_recipient_service_external_state.h" #include "mongo/db/s/resharding/resharding_service_test_helpers.h" -#include "mongo/db/s/sharding_ddl_util.h" #include "mongo/db/service_context.h" #include "mongo/idl/server_parameter_test_util.h" #include "mongo/logv2/log.h" @@ -283,8 +282,7 @@ public: const ReshardingRecipientDocument& recipientDoc) { CollectionOptions options; options.uuid = recipientDoc.getSourceUUID(); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent(opCtx, - recipientDoc.getSourceNss()); + resharding::data_copy::ensureCollectionDropped(opCtx, recipientDoc.getSourceNss()); resharding::data_copy::ensureCollectionExists(opCtx, recipientDoc.getSourceNss(), options); } @@ -292,8 +290,7 @@ public: const ReshardingRecipientDocument& recipientDoc) { CollectionOptions options; options.uuid = recipientDoc.getReshardingUUID(); - mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent( - opCtx, recipientDoc.getTempReshardingNss()); + resharding::data_copy::ensureCollectionDropped(opCtx, recipientDoc.getTempReshardingNss()); resharding::data_copy::ensureCollectionExists( opCtx, recipientDoc.getTempReshardingNss(), options); } diff --git a/src/mongo/db/s/sharding_ddl_util.cpp b/src/mongo/db/s/sharding_ddl_util.cpp index 5ce1ffe27a3..64571ae6284 100644 --- a/src/mongo/db/s/sharding_ddl_util.cpp +++ b/src/mongo/db/s/sharding_ddl_util.cpp @@ -872,37 +872,5 @@ BSONObj getCriticalSectionReasonForRename(const NamespaceString& from, const Nam << "rename" << "from" << from.toString() << "to" << to.toString()); } - -void ensureCollectionDroppedNoChangeEvent(OperationContext* opCtx, - const NamespaceString& nss, - const boost::optional& uuid) { - invariant(!opCtx->lockState()->isLocked()); - invariant(!opCtx->lockState()->inAWriteUnitOfWork()); - - writeConflictRetry(opCtx, - "mongo::sharding_ddl_util::ensureCollectionDroppedNoChangeEvent", - nss.toString(), - [&] { - try { - AutoGetCollection coll(opCtx, nss, MODE_X); - if (!coll || (uuid && coll->uuid() != uuid)) { - // If the collection doesn't exist or exists with a different - // UUID, then the requested collection has been dropped already. - return; - } - - WriteUnitOfWork wuow(opCtx); - uassertStatusOK(coll.getDb()->dropCollectionEvenIfSystem( - opCtx, nss, {} /* dropOpTime */, true /* markFromMigrate */)); - wuow.commit(); - } catch (const ExceptionFor&) { - // AutoGetCollection may raise this exception when the collection - // doesn't exist and the CollectionCatalog starts looking into the - // list of existing views; the error can be ignored, and the - // collection may be considered as already dropped. - return; - } - }); -} } // namespace sharding_ddl_util } // namespace mongo diff --git a/src/mongo/db/s/sharding_ddl_util.h b/src/mongo/db/s/sharding_ddl_util.h index 09ef0a22490..cc800b04632 100644 --- a/src/mongo/db/s/sharding_ddl_util.h +++ b/src/mongo/db/s/sharding_ddl_util.h @@ -221,15 +221,5 @@ void sendDropCollectionParticipantCommandToShards(OperationContext* opCtx, BSONObj getCriticalSectionReasonForRename(const NamespaceString& from, const NamespaceString& to); -/** - * Drops the specified collection or returns without error if the collection has already been - * dropped. A particular incarnation of the collection can be dropped by specifying its UUID. - * - * This functions assumes the collection being dropped doesn't have any two-phase index builds - * active on it. - */ -void ensureCollectionDroppedNoChangeEvent(OperationContext* opCtx, - const NamespaceString& nss, - const boost::optional& uuid = boost::none); } // namespace sharding_ddl_util } // namespace mongo -- cgit v1.2.1