SERVER-42441 renameCollectionForApplyOps should always rename the target out of the way if it exists

Author: xy24

jstests/replsets/apply_ops_idempotency.js

@@ -144,6 +144,29 @@ var tests = {
             mydb.adminCommand({renameCollection: z.getFullName(), to: y.getFullName()}));
         return [mydb, otherdb];
     },
+    renameCollectionAcrossDatabasesWithDropAndConvertToCapped: (db1) => {
+        let db2 = db1.getSiblingDB(db1 + '_');
+        assert.commandWorked(db1.createCollection("a"));
+        assert.commandWorked(db1.createCollection("b"));
+        assert.commandWorked(db2.createCollection("c"));
+        assert.commandWorked(db2.createCollection("d"));
+        let [a, b] = getCollections(db1, ['a', 'b']);
+        let [c, d] = getCollections(db2, ['c', 'd']);
+
+        assert.commandWorked(db1.adminCommand(
+            {renameCollection: a.getFullName(), to: c.getFullName(), dropTarget: true}));
+
+        assert(d.drop());
+
+        assert.commandWorked(db1.adminCommand(
+            {renameCollection: c.getFullName(), to: d.getFullName(), dropTarget: false}));
+
+        assert.commandWorked(db1.adminCommand(
+            {renameCollection: b.getFullName(), to: c.getFullName(), dropTarget: false}));
+        assert.commandWorked(db2.runCommand({convertToCapped: "d", size: 1000}));
+
+        return [db1, db2];
+    },
     createIndex: (mydb) => {
         let [x, y] = getCollections(mydb, ['x', 'y']);
         assert.commandWorked(x.createIndex({x: 1}));

src/mongo/db/catalog/rename_collection.cpp

@@ -89,7 +89,8 @@ bool isReplicatedChanged(OperationContext* opCtx,
 Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
                                       const NamespaceString& source,
                                       const NamespaceString& target,
-                                      RenameCollectionOptions options) {
+                                      RenameCollectionOptions options,
+                                      bool targetExistsAllowed) {
 
     auto replCoord = repl::ReplicationCoordinator::get(opCtx);
     if (opCtx->writesAreReplicated() && !replCoord->canAcceptWritesFor(opCtx, source))
@@ -129,7 +130,7 @@ Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
         if (isCollectionSharded(opCtx, target))
             return {ErrorCodes::IllegalOperation, "cannot rename to a sharded collection"};
 
-        if (!options.dropTarget)
+        if (!targetExistsAllowed && !options.dropTarget)
             return Status(ErrorCodes::NamespaceExists, "target namespace exists");
     }
 
@@ -295,7 +296,8 @@ Status renameCollectionWithinDB(OperationContext* opCtx,
         sourceLock.emplace(opCtx, source, MODE_X);
     }
 
-    auto status = checkSourceAndTargetNamespaces(opCtx, source, target, options);
+    auto status = checkSourceAndTargetNamespaces(
+        opCtx, source, target, options, /* targetExistsAllowed */ false);
     if (!status.isOK())
         return status;
 
@@ -334,7 +336,8 @@ Status renameCollectionWithinDBForApplyOps(OperationContext* opCtx,
         dss->checkDbVersion(opCtx, dssLock);
     }
 
-    auto status = checkSourceAndTargetNamespaces(opCtx, source, target, options);
+    auto status = checkSourceAndTargetNamespaces(
+        opCtx, source, target, options, /* targetExistsAllowed */ true);
     if (!status.isOK())
         return status;
 
@@ -371,7 +374,7 @@ Status renameCollectionWithinDBForApplyOps(OperationContext* opCtx,
                 return Status::OK();
             });
         }
-        if (uuidToDrop && uuidToDrop != targetColl->uuid()) {
+        if (!uuidToDrop || (uuidToDrop && uuidToDrop != targetColl->uuid())) {
             // We need to rename the targetColl to a temporary name.
             auto status = renameTargetCollectionToTmp(
                 opCtx, source, sourceColl->uuid(), db, target, targetColl->uuid());

src/mongo/db/catalog/rename_collection_test.cpp

@@ -765,10 +765,10 @@ TEST_F(RenameCollectionTest,
     _opObserver->renameOpTime = {};
 
     _createCollection(_opCtx.get(), _sourceNss);
-    _createCollection(_opCtx.get(), _targetNss);
+    auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), _targetNss);
     auto dbName = _sourceNss.db().toString();
     auto cmd = BSON("renameCollection" << _sourceNss.ns() << "to" << _targetNss.ns() << "dropTarget"
-                                       << true);
+                                       << dropTargetUUID);
 
     repl::OpTime renameOpTime = {Timestamp(Seconds(200), 1U), 1LL};
     ASSERT_OK(renameCollectionForApplyOps(_opCtx.get(), dbName, boost::none, cmd, renameOpTime));
@@ -787,10 +787,10 @@ DEATH_TEST_F(RenameCollectionTest,
     ASSERT_FALSE(_opCtx->writesAreReplicated());
 
     _createCollection(_opCtx.get(), _sourceNss);
-    _createCollection(_opCtx.get(), _targetNss);
+    auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), _targetNss);
     auto dbName = _sourceNss.db().toString();
     auto cmd = BSON("renameCollection" << _sourceNss.ns() << "to" << _targetNss.ns() << "dropTarget"
-                                       << true);
+                                       << dropTargetUUID);
 
     repl::OpTime renameOpTime = {Timestamp(Seconds(200), 1U), 1LL};
     ASSERT_OK(renameCollectionForApplyOps(_opCtx.get(), dbName, boost::none, cmd, renameOpTime));