Skip to content

Commit 351205d

Browse files
xy24xy24
committed
Revert "SERVER-42441 renameCollectionForApplyOps should always rename the target out of the way if it exists"
This reverts commit dd29b01.
1 parent f2dd217 commit 351205d

File tree

3 files changed

+9
-55
lines changed

3 files changed

+9
-55
lines changed

‎jstests/replsets/apply_ops_idempotency.js‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -144,29 +144,6 @@ var tests = {
144144
mydb.adminCommand({renameCollection: z.getFullName(), to: y.getFullName()}));
145145
return [mydb, otherdb];
146146
},
147-
renameCollectionAcrossDatabasesWithDropAndConvertToCapped: (db1) => {
148-
let db2 = db1.getSiblingDB(db1 + '_');
149-
assert.commandWorked(db1.createCollection("a"));
150-
assert.commandWorked(db1.createCollection("b"));
151-
assert.commandWorked(db2.createCollection("c"));
152-
assert.commandWorked(db2.createCollection("d"));
153-
let [a, b] = getCollections(db1, ['a', 'b']);
154-
let [c, d] = getCollections(db2, ['c', 'd']);
155-
156-
assert.commandWorked(db1.adminCommand(
157-
{renameCollection: a.getFullName(), to: c.getFullName(), dropTarget: true}));
158-
159-
assert(d.drop());
160-
161-
assert.commandWorked(db1.adminCommand(
162-
{renameCollection: c.getFullName(), to: d.getFullName(), dropTarget: false}));
163-
164-
assert.commandWorked(db1.adminCommand(
165-
{renameCollection: b.getFullName(), to: c.getFullName(), dropTarget: false}));
166-
assert.commandWorked(db2.runCommand({convertToCapped: "d", size: 1000}));
167-
168-
return [db1, db2];
169-
},
170147
createIndex: (mydb) => {
171148
let [x, y] = getCollections(mydb, ['x', 'y']);
172149
assert.commandWorked(x.createIndex({x: 1}));

‎src/mongo/db/catalog/rename_collection.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ bool isReplicatedChanged(OperationContext* opCtx,
8989
Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
9090
const NamespaceString& source,
9191
const NamespaceString& target,
92-
RenameCollectionOptions options,
93-
bool targetExistsAllowed) {
92+
RenameCollectionOptions options) {
9493

9594
auto replCoord = repl::ReplicationCoordinator::get(opCtx);
9695
if (opCtx->writesAreReplicated() && !replCoord->canAcceptWritesFor(opCtx, source))
@@ -130,7 +129,7 @@ Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
130129
if (isCollectionSharded(opCtx, target))
131130
return {ErrorCodes::IllegalOperation, "cannot rename to a sharded collection"};
132131

133-
if (!targetExistsAllowed && !options.dropTarget)
132+
if (!options.dropTarget)
134133
return Status(ErrorCodes::NamespaceExists, "target namespace exists");
135134
}
136135

@@ -296,8 +295,7 @@ Status renameCollectionWithinDB(OperationContext* opCtx,
296295
sourceLock.emplace(opCtx, source, MODE_X);
297296
}
298297

299-
auto status = checkSourceAndTargetNamespaces(
300-
opCtx, source, target, options, /* targetExistsAllowed */ false);
298+
auto status = checkSourceAndTargetNamespaces(opCtx, source, target, options);
301299
if (!status.isOK())
302300
return status;
303301

@@ -336,8 +334,7 @@ Status renameCollectionWithinDBForApplyOps(OperationContext* opCtx,
336334
dss->checkDbVersion(opCtx, dssLock);
337335
}
338336

339-
auto status = checkSourceAndTargetNamespaces(
340-
opCtx, source, target, options, /* targetExistsAllowed */ true);
337+
auto status = checkSourceAndTargetNamespaces(opCtx, source, target, options);
341338
if (!status.isOK())
342339
return status;
343340

@@ -374,7 +371,7 @@ Status renameCollectionWithinDBForApplyOps(OperationContext* opCtx,
374371
return Status::OK();
375372
});
376373
}
377-
if (!uuidToDrop || (uuidToDrop && uuidToDrop != targetColl->uuid())) {
374+
if (uuidToDrop && uuidToDrop != targetColl->uuid()) {
378375
// We need to rename the targetColl to a temporary name.
379376
auto status = renameTargetCollectionToTmp(
380377
opCtx, source, sourceColl->uuid(), db, target, targetColl->uuid());

‎src/mongo/db/catalog/rename_collection_test.cpp‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -765,10 +765,10 @@ TEST_F(RenameCollectionTest,
765765
_opObserver->renameOpTime = {};
766766

767767
_createCollection(_opCtx.get(), _sourceNss);
768-
auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), _targetNss);
768+
_createCollection(_opCtx.get(), _targetNss);
769769
auto dbName = _sourceNss.db().toString();
770770
auto cmd = BSON("renameCollection" << _sourceNss.ns() << "to" << _targetNss.ns() << "dropTarget"
771-
<< dropTargetUUID);
771+
<< true);
772772

773773
repl::OpTime renameOpTime = {Timestamp(Seconds(200), 1U), 1LL};
774774
ASSERT_OK(renameCollectionForApplyOps(_opCtx.get(), dbName, boost::none, cmd, renameOpTime));
@@ -787,10 +787,10 @@ DEATH_TEST_F(RenameCollectionTest,
787787
ASSERT_FALSE(_opCtx->writesAreReplicated());
788788

789789
_createCollection(_opCtx.get(), _sourceNss);
790-
auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), _targetNss);
790+
_createCollection(_opCtx.get(), _targetNss);
791791
auto dbName = _sourceNss.db().toString();
792792
auto cmd = BSON("renameCollection" << _sourceNss.ns() << "to" << _targetNss.ns() << "dropTarget"
793-
<< dropTargetUUID);
793+
<< true);
794794

795795
repl::OpTime renameOpTime = {Timestamp(Seconds(200), 1U), 1LL};
796796
ASSERT_OK(renameCollectionForApplyOps(_opCtx.get(), dbName, boost::none, cmd, renameOpTime));
@@ -824,20 +824,11 @@ TEST_F(RenameCollectionTest, RenameCollectionForApplyOpsDropTargetByUUIDEvenIfSo
824824
auto dropTargetNss = NamespaceString("test.bar3");
825825
_createCollectionWithUUID(_opCtx.get(), _targetNss);
826826
auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), dropTargetNss);
827-
//<<<<<<< HEAD
828827
auto uuid = UUID::gen();
829828
auto cmd = BSON("renameCollection" << missingSourceNss.ns() << "to" << _targetNss.ns()
830829
<< "dropTarget" << dropTargetUUID);
831830
ASSERT_OK(
832831
renameCollectionForApplyOps(_opCtx.get(), missingSourceNss.db().toString(), uuid, cmd, {}));
833-
//=======
834-
// auto uuidDoc = BSON("ui" << UUID::gen());
835-
// auto cmd = BSON("renameCollection" << missingSourceNss.ns() << "to" << _targetNss.ns()
836-
// << "dropTarget" << dropTargetUUID);
837-
// ASSERT_OK(renameCollectionForApplyOps(
838-
// _opCtx.get(), missingSourceNss.db().toString(), uuidDoc["ui"], cmd, {}));
839-
//>>>>>>> SERVER-42441 renameCollectionForApplyOps should always rename the target out of the
840-
// way if it exists
841832
ASSERT_TRUE(_collectionExists(_opCtx.get(), _targetNss));
842833
ASSERT_FALSE(_collectionExists(_opCtx.get(), dropTargetNss));
843834
}
@@ -872,12 +863,7 @@ TEST_F(RenameCollectionTest, RenameCollectionForApplyOpsDropTargetByUUIDEvenIfSo
872863
_createCollectionWithUUID(_opCtx.get(), _targetNss);
873864

874865
auto dropTargetUUID = _createCollectionWithUUID(_opCtx.get(), dropTargetNss);
875-
//<<<<<<< HEAD
876866
auto uuid = _createCollectionWithUUID(_opCtx.get(), dropPendingNss);
877-
//=======
878-
// auto uuidDoc = BSON("ui" << _createCollectionWithUUID(_opCtx.get(), dropPendingNss));
879-
//>>>>>>> SERVER-42441 renameCollectionForApplyOps should always rename the target out of the
880-
// way if it exists
881867
auto cmd = BSON("renameCollection" << dropPendingNss.ns() << "to" << _targetNss.ns()
882868
<< "dropTarget" << dropTargetUUID);
883869

@@ -1012,14 +998,8 @@ void _testRenameCollectionAcrossDatabaseOplogEntries(
1012998
if (forApplyOps) {
1013999
auto cmd = BSON("renameCollection" << sourceNss.ns() << "to" << targetNss.ns()
10141000
<< "dropTarget" << true);
1015-
//<<<<<<< HEAD
10161001
ASSERT_OK(
10171002
renameCollectionForApplyOps(opCtx, sourceNss.db().toString(), boost::none, cmd, {}));
1018-
//=======
1019-
// ASSERT_OK(renameCollectionForApplyOps(opCtx, sourceNss.db().toString(), {}, cmd,
1020-
// {}));
1021-
//>>>>>>> SERVER-42441 renameCollectionForApplyOps should always rename the target out of
1022-
// the way if it exists
10231003
} else {
10241004
RenameCollectionOptions options;
10251005
options.dropTarget = true;

0 commit comments

Comments
 (0)