SERVER-72235 Core Server Avoid the usage of dropCollectionEvenIfSystem() outside of resharding code

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>& 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
@@ -60,6 +60,17 @@ void ensureCollectionExists(OperationContext* opCtx,
                             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>& 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
  * collections and conflict stash collections that are associated with the in-progress resharding
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>& 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<ErrorCodes::InvalidViewDefinition>&) {
-                               // 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>& uuid = boost::none);
 }  // namespace sharding_ddl_util
 }  // namespace mongo