SERVER-69874 Execute in dryRun mode first during unique index conversion on a sharded collection to ensure consistent index specs across shards

(cherry picked from commit 78131d8e3da7114a037d55add0483a82a5133bd8)

6 files changed, 266 insertions, 0 deletions

diff --git a/jstests/noPassthrough/collmod_convert_to_unique_sharded.js b/jstests/noPassthrough/collmod_convert_to_unique_sharded.js
new file mode 100644
index 00000000000..0a02ffd1c1e
--- /dev/null
+++ b/jstests/noPassthrough/collmod_convert_to_unique_sharded.js
@@ -0,0 +1,91 @@
+/**
+ * Tests collMod unique conversion ensures consistent index specs across all shards.
+ *
+ * @tags: [
+ *   # TODO(SERVER-61182): Fix WiredTigerKVEngine::alterIdentMetadata() under inMemory.
+ *   requires_persistence,
+ *   requires_sharding,
+ * ]
+ */
+(function() {
+'use strict';
+
+function countUniqueIndexes(coll, key) {
+    const all = coll.getIndexes().filter(function(z) {
+        return z.unique && friendlyEqual(z.key, key);
+    });
+    return all.length;
+}
+
+function countPrepareUniqueIndexes(coll, key) {
+    const all = coll.getIndexes().filter(function(z) {
+        return z.prepareUnique && friendlyEqual(z.key, key);
+    });
+    return all.length;
+}
+
+const st = new ShardingTest({shards: 2});
+const mongos = st.s;
+
+const db = mongos.getDB(jsTestName());
+assert.commandWorked(
+    mongos.adminCommand({enableSharding: db.getName(), primaryShard: st.shard0.name}));
+
+const shardedColl = db.sharded;
+
+assert.commandWorked(
+    mongos.adminCommand({shardCollection: shardedColl.getFullName(), key: {a: 1}}));
+
+// Move {a: 1} to shard0 and {a: 2} to shard1.
+assert.commandWorked(st.splitAt(shardedColl.getFullName(), {a: 2}));
+assert.commandWorked(mongos.adminCommand(
+    {moveChunk: shardedColl.getFullName(), find: {a: 1}, to: st.shard0.shardName}));
+assert.commandWorked(mongos.adminCommand(
+    {moveChunk: shardedColl.getFullName(), find: {a: 2}, to: st.shard1.shardName}));
+
+assert.commandWorked(shardedColl.createIndex({a: 1}));
+
+assert.commandWorked(shardedColl.insert({_id: 0, a: 1}));
+assert.commandWorked(shardedColl.insert({_id: 1, a: 2}));
+assert.commandWorked(shardedColl.insert({_id: 2, a: 2}));
+
+// Setting the indexes to 'prepareUnique' ensures no new duplicates will be inserted.
+assert.commandWorked(db.runCommand(
+    {collMod: shardedColl.getName(), index: {keyPattern: {a: 1}, prepareUnique: true}}));
+assert.commandFailedWithCode(shardedColl.insert({_id: 3, a: 1}), ErrorCodes.DuplicateKey);
+assert.commandFailedWithCode(shardedColl.insert({_id: 4, a: 2}), ErrorCodes.DuplicateKey);
+
+// Try converting the index to unique and make sure no indexes are converted on any shards.
+assert.commandFailedWithCode(
+    db.runCommand(
+        {collMod: shardedColl.getName(), index: {keyPattern: {a: 1}, hidden: true, unique: true}}),
+    ErrorCodes.CannotConvertIndexToUnique);
+
+const s0Coll = st.shard0.getDB(jsTestName()).getCollection("sharded");
+const s1Coll = st.shard1.getDB(jsTestName()).getCollection("sharded");
+assert.eq(countUniqueIndexes(s0Coll, {a: 1}),
+          0,
+          'index should not be unique: ' + tojson(s0Coll.getIndexes()));
+assert.eq(countUniqueIndexes(s1Coll, {a: 1}),
+          0,
+          'index should not be unique: ' + tojson(s1Coll.getIndexes()));
+assert.eq(countPrepareUniqueIndexes(s0Coll, {a: 1}),
+          1,
+          'index should be prepareUnique: ' + tojson(s0Coll.getIndexes()));
+assert.eq(countPrepareUniqueIndexes(s1Coll, {a: 1}),
+          1,
+          'index should be prepareUnique: ' + tojson(s1Coll.getIndexes()));
+
+// Remove the duplicate and confirm the indexes are converted.
+assert.commandWorked(shardedColl.deleteOne({_id: 2}));
+assert.commandWorked(db.runCommand(
+    {collMod: shardedColl.getName(), index: {keyPattern: {a: 1}, hidden: true, unique: true}}));
+assert.eq(countUniqueIndexes(s0Coll, {a: 1}),
+          1,
+          'index should be unique: ' + tojson(s0Coll.getIndexes()));
+assert.eq(countUniqueIndexes(s1Coll, {a: 1}),
+          1,
+          'index should be unique: ' + tojson(s1Coll.getIndexes()));
+
+st.stop();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript
index 5c78a61f615..749f345c611 100644
--- a/src/mongo/db/catalog/SConscript
+++ b/src/mongo/db/catalog/SConscript
@@ -659,6 +659,7 @@ if wiredtiger:
             'collection_test.cpp',
             'collection_validation_test.cpp',
             'collection_writer_test.cpp',
+            'coll_mod_test.cpp',
             'commit_quorum_options_test.cpp',
             'create_collection_test.cpp',
             'database_test.cpp',
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index cd8a57def3d..85555c060a5 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -969,6 +969,39 @@ Status _collModInternal(OperationContext* opCtx,
 
 }  // namespace
 
+bool isCollModIndexUniqueConversion(const CollModRequest& request) {
+    auto index = request.getIndex();
+    if (!index) {
+        return false;
+    }
+    if (auto indexUnique = index->getUnique(); !indexUnique) {
+        return false;
+    }
+    // Checks if the request is an actual unique conversion instead of a dry run.
+    if (auto dryRun = request.getDryRun(); dryRun && *dryRun) {
+        return false;
+    }
+    return true;
+}
+
+CollModRequest makeCollModDryRunRequest(const CollModRequest& request) {
+    CollModRequest dryRunRequest;
+    CollModIndex dryRunIndex;
+    const auto& requestIndex = request.getIndex();
+    dryRunIndex.setUnique(true);
+    if (auto keyPattern = requestIndex->getKeyPattern()) {
+        dryRunIndex.setKeyPattern(keyPattern);
+    } else if (auto name = requestIndex->getName()) {
+        dryRunIndex.setName(name);
+    }
+    if (auto uuid = request.getCollectionUUID()) {
+        dryRunRequest.setCollectionUUID(uuid);
+    }
+    dryRunRequest.setIndex(dryRunIndex);
+    dryRunRequest.setDryRun(true);
+    return dryRunRequest;
+}
+
 Status processCollModCommand(OperationContext* opCtx,
                              const NamespaceStringOrUUID& nsOrUUID,
                              const CollMod& cmd,
diff --git a/src/mongo/db/catalog/coll_mod.h b/src/mongo/db/catalog/coll_mod.h
index f08aca11444..f2b0c1702d5 100644
--- a/src/mongo/db/catalog/coll_mod.h
+++ b/src/mongo/db/catalog/coll_mod.h
@@ -48,6 +48,18 @@ class OperationContext;
 void addCollectionUUIDs(OperationContext* opCtx);
 
 /**
+ * Checks if the collMod request is converting an index to unique.
+ */
+bool isCollModIndexUniqueConversion(const CollModRequest& request);
+
+/**
+ * Constructs a valid collMod dry-run request from the original request.
+ * The 'dryRun' option can only be used with the index 'unique' option, so we assume 'request' must
+ * have the 'unique' option. The function will also remove other options from the original request.
+ */
+CollModRequest makeCollModDryRunRequest(const CollModRequest& request);
+
+/**
  * Performs the collection modification described in "cmd" on the collection "ns".
  */
 Status processCollModCommand(OperationContext* opCtx,
diff --git a/src/mongo/db/catalog/coll_mod_test.cpp b/src/mongo/db/catalog/coll_mod_test.cpp
new file mode 100644
index 00000000000..818b0cc8fdb
--- /dev/null
+++ b/src/mongo/db/catalog/coll_mod_test.cpp
@@ -0,0 +1,106 @@
+/**
+ *    Copyright (C) 2022-present MongoDB, Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the Server Side Public License, version 1,
+ *    as published by MongoDB, Inc.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    Server Side Public License for more details.
+ *
+ *    You should have received a copy of the Server Side Public License
+ *    along with this program. If not, see
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the Server Side Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#include "mongo/db/catalog/coll_mod.h"
+
+#include <boost/optional.hpp>
+
+#include "mongo/db/coll_mod_gen.h"
+#include "mongo/unittest/unittest.h"
+
+namespace mongo {
+namespace {
+TEST(CollModOptionTest, isConvertingIndexToUnique) {
+    IDLParserContext ctx("collMod");
+    auto requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true}}");
+    auto request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_TRUE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true, hidden: true}}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_TRUE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson(
+        "{index: {keyPattern: {a: 1}, unique: true, hidden: true}, validationAction: 'warn'}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_TRUE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true}, dryRun: true}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_FALSE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true}, dryRun: false}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_TRUE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, prepareUnique: true}}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_FALSE(isCollModIndexUniqueConversion(request));
+
+    requestObj = fromjson("{validationAction: 'warn'}");
+    request = CollModRequest::parse(ctx, requestObj);
+    ASSERT_FALSE(isCollModIndexUniqueConversion(request));
+}
+
+TEST(CollModOptionTest, makeDryRunRequest) {
+    IDLParserContext ctx("collMod");
+    auto requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true}}");
+    auto request = CollModRequest::parse(ctx, requestObj);
+    auto dryRunRequest = makeCollModDryRunRequest(request);
+    ASSERT_TRUE(dryRunRequest.getIndex()->getKeyPattern()->binaryEqual(fromjson("{a: 1}")));
+    ASSERT_TRUE(dryRunRequest.getIndex()->getUnique() && *dryRunRequest.getIndex()->getUnique());
+    ASSERT_TRUE(dryRunRequest.getDryRun() && *dryRunRequest.getDryRun());
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true, hidden: true}}");
+    request = CollModRequest::parse(ctx, requestObj);
+    dryRunRequest = makeCollModDryRunRequest(request);
+    ASSERT_TRUE(dryRunRequest.getIndex()->getKeyPattern()->binaryEqual(fromjson("{a: 1}")));
+    ASSERT_TRUE(dryRunRequest.getIndex()->getUnique() && *dryRunRequest.getIndex()->getUnique());
+    ASSERT_FALSE(dryRunRequest.getIndex()->getHidden());
+    ASSERT_TRUE(dryRunRequest.getDryRun() && *dryRunRequest.getDryRun());
+
+    requestObj = fromjson(
+        "{index: {keyPattern: {a: 1}, unique: true, hidden: true}, validationAction: 'warn'}");
+    request = CollModRequest::parse(ctx, requestObj);
+    dryRunRequest = makeCollModDryRunRequest(request);
+    ASSERT_TRUE(dryRunRequest.getIndex()->getKeyPattern()->binaryEqual(fromjson("{a: 1}")));
+    ASSERT_TRUE(dryRunRequest.getIndex()->getUnique() && *dryRunRequest.getIndex()->getUnique());
+    ASSERT_FALSE(dryRunRequest.getIndex()->getHidden());
+    ASSERT_FALSE(dryRunRequest.getValidationAction());
+    ASSERT_TRUE(dryRunRequest.getDryRun() && *dryRunRequest.getDryRun());
+
+    requestObj = fromjson("{index: {keyPattern: {a: 1}, unique: true}, dryRun: false}");
+    request = CollModRequest::parse(ctx, requestObj);
+    dryRunRequest = makeCollModDryRunRequest(request);
+    ASSERT_TRUE(dryRunRequest.getIndex()->getKeyPattern()->binaryEqual(fromjson("{a: 1}")));
+    ASSERT_TRUE(dryRunRequest.getIndex()->getUnique() && *dryRunRequest.getIndex()->getUnique());
+    ASSERT_TRUE(dryRunRequest.getDryRun() && *dryRunRequest.getDryRun());
+}
+}  // namespace
+}  // namespace mongo
diff --git a/src/mongo/db/s/collmod_coordinator.cpp b/src/mongo/db/s/collmod_coordinator.cpp
index 3e5abedde21..318d24b6c72 100644
--- a/src/mongo/db/s/collmod_coordinator.cpp
+++ b/src/mongo/db/s/collmod_coordinator.cpp
@@ -30,6 +30,7 @@
 
 #include "mongo/db/s/collmod_coordinator.h"
 
+#include "mongo/db/catalog/coll_mod.h"
 #include "mongo/db/catalog/collection_catalog.h"
 #include "mongo/db/catalog/collection_uuid_mismatch.h"
 #include "mongo/db/catalog/database_holder.h"
@@ -291,6 +292,28 @@ ExecutorFuture<void> CollModCoordinator::_runImpl(
                     auto primaryShardOwningChunk = std::find(shardsOwningChunks.begin(),
                                                              shardsOwningChunks.end(),
                                                              _shardingInfo->primaryShard);
+
+                    // If trying to convert an index to unique, executes a dryRun first to find any
+                    // duplicates without actually changing the indexes to avoid inconsistent index
+                    // specs on different shards.
+                    // Example:
+                    //   Shard0: {_id: 0, a: 1}
+                    //   Shard1: {_id: 1, a: 2}, {_id: 2, a: 2}
+                    //   When trying to convert index {a: 1} to unique, the dry run will return the
+                    //   duplicate errors to the user without converting the indexes.
+                    if (isCollModIndexUniqueConversion(_request)) {
+                        // The 'dryRun' option only works with 'unique' index option. We need to
+                        // strip out other incompatible options.
+                        auto dryRunRequest = ShardsvrCollModParticipant{
+                            originalNss(), makeCollModDryRunRequest(_request)};
+                        sharding_ddl_util::sendAuthenticatedCommandToShards(
+                            opCtx,
+                            nss().db(),
+                            CommandHelpers::appendMajorityWriteConcern(dryRunRequest.toBSON({})),
+                            shardsOwningChunks,
+                            **executor);
+                    }
+
                     // A view definition will only be present on the primary shard. So we pass an
                     // addition 'performViewChange' flag only to the primary shard.
                     if (primaryShardOwningChunk != shardsOwningChunks.end()) {