SERVER-79661 Make the 'internalRenameIfOptionsAndIndexesMatch' command sharding-aware

Author: jordist

jstests/noPassthrough/internal_rename_if_options_and_indexes_match_sharding.js

@@ -0,0 +1,120 @@
+/*
+ * Tests the internalRenameIfOptionsAndIndexesMatch on shard servers.
+ * Note: Because the mongos doesn't expose this command, we need a directed noPassthrough test to be
+ * able to test it by running it directly on shards.
+ */
+
+const st = new ShardingTest({shards: 2});
+
+const dbName = "test";
+const testDB = st.getDB(dbName);
+const sourceColl = testDB['source'];
+const destColl = testDB['dest'];
+const inexistentColl = testDB['inexistentColl'];
+
+function setupCollections() {
+    sourceColl.drop();
+    destColl.drop();
+    inexistentColl.drop();
+
+    sourceColl.insert({x: 1});
+    destColl.insert({});
+}
+
+setupCollections();
+
+const dbPrimaryShardConn = st.getPrimaryShard(dbName);
+
+function makeCorrectCommand(sourceColl, destColl) {
+    const dbVersion = st.s.getDB('config')['databases'].findOne({_id: dbName}).version;
+
+    let collectionInfos = testDB.getCollectionInfos({name: destColl.getName()});
+    let collectionOptions = collectionInfos.length === 1 ? collectionInfos[0].options : {};
+
+    return {
+        internalRenameIfOptionsAndIndexesMatch: 1,
+        from: sourceColl.getFullName(),
+        to: destColl.getFullName(),
+        indexes: destColl.getIndexes(),
+        collectionOptions: collectionOptions,
+        databaseVersion: dbVersion
+    };
+}
+
+// If no dbVersion attached, command fails.
+{
+    let cmd = makeCorrectCommand(sourceColl, destColl);
+    delete cmd.databaseVersion;
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.IllegalOperation);
+}
+
+// If wrong dbVersion attached, command fails
+{
+    let cmd = makeCorrectCommand(sourceColl, destColl);
+    cmd.databaseVersion.timestamp = Timestamp(12345, 6789);
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.StaleDbVersion);
+}
+
+// If source collection does not exist, command fails.
+{
+    let cmd = makeCorrectCommand(inexistentColl, destColl);
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd),
+                                 ErrorCodes.NamespaceNotFound);
+}
+
+// If expected indexes don't match, command fails.
+{
+    destColl.createIndex({x: 1});
+
+    // Collection does not exist, but indexes expected.
+    let cmd = makeCorrectCommand(sourceColl, inexistentColl);
+    cmd.indexes = destColl.getIndexes();
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.CommandFailed);
+
+    // Collection exists and has indexes, but wrong indexes expected.
+    cmd = makeCorrectCommand(sourceColl, destColl);
+    destColl.dropIndex("x_1");
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.CommandFailed);
+}
+setupCollections();
+
+// If expected options don't match, command fails.
+{
+    assert.commandWorked(testDB.runCommand({collMod: destColl.getName(), validator: {a: 1}}));
+    const options = testDB.getCollectionInfos({name: destColl.getName()})[0];
+
+    // Collection does not exist, but some options expected.
+    let cmd = makeCorrectCommand(sourceColl, inexistentColl);
+    cmd.collectionOptions = options;
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.CommandFailed);
+
+    // Collection exists and has options set, but wrong options expected.
+    cmd = makeCorrectCommand(sourceColl, destColl);
+    assert.commandWorked(testDB.runCommand({collMod: destColl.getName(), validator: {}}));
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.CommandFailed);
+}
+setupCollections();
+
+// If target collection is sharded, fails
+{
+    let shardedColl = testDB['sharded'];
+    assert.commandWorked(
+        st.s.adminCommand({shardCollection: shardedColl.getFullName(), key: {x: 1}}));
+    let cmd = makeCorrectCommand(sourceColl, shardedColl);
+    assert.commandFailedWithCode(dbPrimaryShardConn.adminCommand(cmd), ErrorCodes.IllegalOperation);
+}
+setupCollections();
+
+// Expectations are met. Command works.
+{
+    let cmd = makeCorrectCommand(sourceColl, destColl);
+    assert.commandWorked(dbPrimaryShardConn.adminCommand(cmd));
+    assert.eq(1, destColl.find({x: 1}).itcount());
+
+    setupCollections();
+    cmd = makeCorrectCommand(sourceColl, inexistentColl);
+    assert.commandWorked(dbPrimaryShardConn.adminCommand(cmd));
+    assert.eq(1, inexistentColl.find({x: 1}).itcount());
+}
+
+st.stop();

jstests/noPassthrough/timeseries/timeseries_out_concurrent_sharding.js

@@ -800,41 +800,52 @@ void doLocalRenameIfOptionsAndIndexesHaveNotChanged(OperationContext* opCtx,
                                                     std::list<BSONObj> originalIndexes,
                                                     BSONObj originalCollectionOptions) {
     AutoGetDb dbLock(opCtx, targetNs.dbName(), MODE_X);
+
+    // Check target collection options match expected.
     auto collection = dbLock.getDb()
         ? CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, targetNs)
         : nullptr;
-    BSONObj collectionOptions = {};
-    if (collection) {
-        // We do not include the UUID field in the options comparison. It is ok if the target
-        // collection was dropped and recreated, as long as the new target collection has the same
-        // options and indexes as the original one did. This is mainly to support concurrent $out
-        // to the same collection.
-        collectionOptions = collection->getCollectionOptions().toBSON().removeField("uuid");
-    }
+    const BSONObj collectionOptions =
+        collection ? collection->getCollectionOptions().toBSON() : BSONObj();
+    checkTargetCollectionOptionsMatch(targetNs, originalCollectionOptions, collectionOptions);
+
+    // Check target collection indexes match expected.
+    const auto currentIndexes =
+        listIndexesEmptyListIfMissing(opCtx, targetNs, ListIndexesInclude::Nothing);
+    checkTargetCollectionIndexesMatch(targetNs, originalIndexes, currentIndexes);
+
+    validateAndRunRenameCollection(opCtx, sourceNs, targetNs, options);
+}
 
+void checkTargetCollectionOptionsMatch(const NamespaceString& targetNss,
+                                       const BSONObj& expectedOptions,
+                                       const BSONObj& currentOptions) {
+    // We do not include the UUID field in the options comparison. It is ok if the target collection
+    // was dropped and recreated, as long as the new target collection has the same options and
+    // indexes as the original one did. This is mainly to support concurrent $out to the same
+    // collection.
     uassert(ErrorCodes::CommandFailed,
             str::stream() << "collection options of target collection "
-                          << targetNs.toStringForErrorMsg()
-                          << " changed during processing. Original options: "
-                          << originalCollectionOptions << ", new options: " << collectionOptions,
-            SimpleBSONObjComparator::kInstance.evaluate(
-                originalCollectionOptions.removeField("uuid") == collectionOptions));
-
-    auto currentIndexes =
-        listIndexesEmptyListIfMissing(opCtx, targetNs, ListIndexesInclude::Nothing);
+                          << targetNss.toStringForErrorMsg()
+                          << " changed during processing. Original options: " << expectedOptions
+                          << ", new options: " << currentOptions,
+            SimpleBSONObjComparator::kInstance.evaluate(expectedOptions.removeField("uuid") ==
+                                                        currentOptions.removeField("uuid")));
+}
 
+void checkTargetCollectionIndexesMatch(const NamespaceString& targetNss,
+                                       const std::list<BSONObj>& expectedIndexes,
+                                       const std::list<BSONObj>& currentIndexes) {
     UnorderedFieldsBSONObjComparator comparator;
     uassert(
         ErrorCodes::CommandFailed,
-        str::stream() << "indexes of target collection " << targetNs.toStringForErrorMsg()
+        str::stream() << "indexes of target collection " << targetNss.toStringForErrorMsg()
                       << " changed during processing.",
-        originalIndexes.size() == currentIndexes.size() &&
-            std::equal(originalIndexes.begin(),
-                       originalIndexes.end(),
+        expectedIndexes.size() == currentIndexes.size() &&
+            std::equal(expectedIndexes.begin(),
+                       expectedIndexes.end(),
                        currentIndexes.begin(),
                        [&](auto& lhs, auto& rhs) { return comparator.compare(lhs, rhs) == 0; }));
-
-    validateAndRunRenameCollection(opCtx, sourceNs, targetNs, options);
 }
 
 void validateNamespacesForRenameCollection(OperationContext* opCtx,