SERVER-52778 Cleanup the legacy shardCollection code path

13 files changed, 51 insertions, 90 deletions

diff --git a/src/mongo/client/SConscript b/src/mongo/client/SConscript
index e6176fc576c..e120354b79c 100644
--- a/src/mongo/client/SConscript
+++ b/src/mongo/client/SConscript
@@ -195,7 +195,6 @@ clientDriverEnv.Library(
         '$BUILD_DIR/mongo/db/wire_version',
         '$BUILD_DIR/mongo/rpc/command_status',
         '$BUILD_DIR/mongo/rpc/rpc',
-        '$BUILD_DIR/mongo/s/common_s',
         'authentication',
         'client_query',
         'connection_string',
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript
index 6732fc7e951..e88b0b70c17 100644
--- a/src/mongo/db/SConscript
+++ b/src/mongo/db/SConscript
@@ -1242,7 +1242,6 @@ env.Library(
         '$BUILD_DIR/mongo/db/auth/auth_checks',
         '$BUILD_DIR/mongo/db/index/index_access_methods',
         '$BUILD_DIR/mongo/db/s/resharding_util',
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/scripting/scripting',
         '$BUILD_DIR/mongo/util/background_job',
         '$BUILD_DIR/mongo/util/elapsed_tracker',
diff --git a/src/mongo/db/pipeline/SConscript b/src/mongo/db/pipeline/SConscript
index 6baca8e610b..852e23217da 100644
--- a/src/mongo/db/pipeline/SConscript
+++ b/src/mongo/db/pipeline/SConscript
@@ -174,7 +174,6 @@ env.Library(
     LIBDEPS=[
         '$BUILD_DIR/mongo/s/async_requests_sender',
         '$BUILD_DIR/mongo/s/commands/shared_cluster_commands',
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/s/query/cluster_query',
         'aggregation',
     ],
diff --git a/src/mongo/db/pipeline/process_interface/SConscript b/src/mongo/db/pipeline/process_interface/SConscript
index 4ead2683ec0..22c7f1dfeda 100644
--- a/src/mongo/db/pipeline/process_interface/SConscript
+++ b/src/mongo/db/pipeline/process_interface/SConscript
@@ -64,7 +64,6 @@ env.Library(
         'mongod_process_interfaces',
     ],
     LIBDEPS_PRIVATE=[
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/s/sharding_api',
     ],
 )
diff --git a/src/mongo/db/s/SConscript b/src/mongo/db/s/SConscript
index 3d0d433bb21..102668c520f 100644
--- a/src/mongo/db/s/SConscript
+++ b/src/mongo/db/s/SConscript
@@ -175,7 +175,6 @@ env.Library(
         '$BUILD_DIR/mongo/db/pipeline/pipeline',
         '$BUILD_DIR/mongo/db/storage/write_unit_of_work',
         '$BUILD_DIR/mongo/s/async_requests_sender',
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/s/grid',
         'sharding_api_d',
     ],
@@ -234,7 +233,6 @@ env.Library(
     LIBDEPS=[
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/bson/util/bson_extract',
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/s/write_ops/batch_write_types',
     ],
 )
diff --git a/src/mongo/db/s/config/configsvr_refine_collection_shard_key_command.cpp b/src/mongo/db/s/config/configsvr_refine_collection_shard_key_command.cpp
index 53ccc365113..d8f90700952 100644
--- a/src/mongo/db/s/config/configsvr_refine_collection_shard_key_command.cpp
+++ b/src/mongo/db/s/config/configsvr_refine_collection_shard_key_command.cpp
@@ -113,11 +113,11 @@ public:
                               << "refineCollectionShardKey namespace " << nss << " is not sharded");
             }
 
-            const auto oldShardKeyPattern = ShardKeyPattern(collType.getKeyPattern());
-            const auto proposedKey = request().getKey().getOwned();
+            const ShardKeyPattern oldShardKeyPattern(collType.getKeyPattern());
+            const ShardKeyPattern newShardKeyPattern(request().getKey());
 
             if (SimpleBSONObjComparator::kInstance.evaluate(oldShardKeyPattern.toBSON() ==
-                                                            proposedKey)) {
+                                                            newShardKeyPattern.toBSON())) {
                 repl::ReplClientInfo::forClient(opCtx->getClient())
                     .setLastOpToSystemLastOpTime(opCtx);
                 return;
@@ -131,12 +131,11 @@ public:
 
             // Validate the given shard key (i) extends the current shard key, (ii) has a "useful"
             // index, and (iii) the index in question has no null entries.
-            const auto newShardKeyPattern = ShardKeyPattern(proposedKey);
-
             uassert(ErrorCodes::InvalidOptions,
-                    str::stream() << "refineCollectionShardKey shard key " << proposedKey.toString()
+                    str::stream() << "refineCollectionShardKey shard key "
+                                  << newShardKeyPattern.toString()
                                   << " does not extend the current shard key "
-                                  << collType.getKeyPattern().toString(),
+                                  << oldShardKeyPattern.toString(),
                     oldShardKeyPattern.isExtendedBy(newShardKeyPattern));
 
             // Indexes are loaded using shard versions, so validating the shard key may need to be
@@ -152,7 +151,6 @@ public:
                                   shardkeyutil::validateShardKeyIndexExistsOrCreateIfPossible(
                                       opCtx,
                                       nss,
-                                      proposedKey,
                                       newShardKeyPattern,
                                       boost::none,
                                       collType.getUnique(),
@@ -164,7 +162,8 @@ public:
                   "CMD: refineCollectionShardKey",
                   "request"_attr = request().toBSON({}));
 
-            audit::logRefineCollectionShardKey(opCtx->getClient(), nss.ns(), proposedKey);
+            audit::logRefineCollectionShardKey(
+                opCtx->getClient(), nss.ns(), newShardKeyPattern.toBSON());
 
             ShardingCatalogManager::get(opCtx)->refineCollectionShardKey(
                 opCtx, nss, newShardKeyPattern);
diff --git a/src/mongo/db/s/create_collection_coordinator.cpp b/src/mongo/db/s/create_collection_coordinator.cpp
index b066f5f05b0..f565463572d 100644
--- a/src/mongo/db/s/create_collection_coordinator.cpp
+++ b/src/mongo/db/s/create_collection_coordinator.cpp
@@ -467,7 +467,6 @@ void CreateCollectionCoordinator::_createCollectionAndIndexes(OperationContext*
     shardkeyutil::validateShardKeyIndexExistsOrCreateIfPossible(
         opCtx,
         _nss,
-        _shardKeyPattern->toBSON(),
         *_shardKeyPattern,
         _collation,
         _request.getUnique() ? *_request.getUnique() : false,
diff --git a/src/mongo/db/s/resharding/resharding_recipient_service.cpp b/src/mongo/db/s/resharding/resharding_recipient_service.cpp
index f46c264c865..bafd83d25f0 100644
--- a/src/mongo/db/s/resharding/resharding_recipient_service.cpp
+++ b/src/mongo/db/s/resharding/resharding_recipient_service.cpp
@@ -416,7 +416,6 @@ void ReshardingRecipientService::RecipientStateMachine::
                               shardkeyutil::validateShardKeyIndexExistsOrCreateIfPossible(
                                   opCtx.get(),
                                   _metadata.getTempReshardingNss(),
-                                  shardKeyPattern.toBSON(),
                                   shardKeyPattern,
                                   CollationSpec::kSimpleSpec,
                                   false,
diff --git a/src/mongo/db/s/shard_collection_legacy.cpp b/src/mongo/db/s/shard_collection_legacy.cpp
index a3731edc6bf..0957ac63103 100644
--- a/src/mongo/db/s/shard_collection_legacy.cpp
+++ b/src/mongo/db/s/shard_collection_legacy.cpp
@@ -202,14 +202,14 @@ void checkCollation(OperationContext* opCtx, const ShardsvrShardCollectionReques
             CollatorInterface::collatorsMatch(requestedCollator.get(), actualCollator));
 }
 
-/**
- * Compares the proposed shard key with the shard key of the collection's existing zones
- * to ensure they are a legal combination.
- */
-void validateShardKeyAgainstExistingZones(OperationContext* opCtx,
-                                          const BSONObj& proposedKey,
-                                          const ShardKeyPattern& shardKeyPattern,
-                                          const std::vector<TagsType>& tags) {
+std::vector<TagsType> getTagsAndValidate(OperationContext* opCtx,
+                                         const NamespaceString& nss,
+                                         const BSONObj& proposedKey) {
+    const auto catalogClient = Grid::get(opCtx)->catalogClient();
+    auto tags = uassertStatusOK(catalogClient->getTagsForCollection(opCtx, nss));
+
+    // Compares the proposed shard key with the shard key of the collection's existing zones to
+    // ensure they are a legal combination
     for (const auto& tag : tags) {
         BSONObjIterator tagMinFields(tag.getMinKey());
         BSONObjIterator tagMaxFields(tag.getMaxKey());
@@ -235,7 +235,7 @@ void validateShardKeyAgainstExistingZones(OperationContext* opCtx,
                                   << tag.getMinKey() << " -->> " << tag.getMaxKey(),
                     match);
 
-            // If the field is hashed, make sure that the min and max values are of supported type.
+            // If the field is hashed, make sure that the min and max values are of supported type
             uassert(
                 ErrorCodes::InvalidOptions,
                 str::stream() << "cannot do hash sharding with the proposed key "
@@ -247,19 +247,6 @@ void validateShardKeyAgainstExistingZones(OperationContext* opCtx,
                      ShardKeyPattern::isValidHashedValue(tagMaxKeyElement)));
         }
     }
-}
-
-std::vector<TagsType> getTagsAndValidate(OperationContext* opCtx,
-                                         const NamespaceString& nss,
-                                         const BSONObj& proposedKey,
-                                         const ShardKeyPattern& shardKeyPattern) {
-    // Read zone info
-    const auto catalogClient = Grid::get(opCtx)->catalogClient();
-    auto tags = uassertStatusOK(catalogClient->getTagsForCollection(opCtx, nss));
-
-    if (!tags.empty()) {
-        validateShardKeyAgainstExistingZones(opCtx, proposedKey, shardKeyPattern, tags);
-    }
 
     return tags;
 }
@@ -302,16 +289,6 @@ boost::optional<UUID> getUUIDFromPrimaryShard(OperationContext* opCtx, const Nam
     return uassertStatusOK(UUID::parse(collectionInfo["uuid"]));
 }
 
-UUID getOrGenerateUUID(OperationContext* opCtx,
-                       const NamespaceString& nss,
-                       const ShardsvrShardCollectionRequest& request) {
-    if (request.getGetUUIDfromPrimaryShard()) {
-        return *getUUIDFromPrimaryShard(opCtx, nss);
-    }
-
-    return UUID::gen();
-}
-
 bool checkIfCollectionIsEmpty(OperationContext* opCtx, const NamespaceString& nss) {
     // Use find with predicate instead of count in order to ensure that the count
     // command doesn't just consult the cached metadata, which may not always be
@@ -329,30 +306,12 @@ int getNumShards(OperationContext* opCtx) {
 ShardCollectionTargetState calculateTargetState(OperationContext* opCtx,
                                                 const NamespaceString& nss,
                                                 const ShardsvrShardCollectionRequest& request) {
-    auto proposedKey(request.getKey().getOwned());
-    ShardKeyPattern shardKeyPattern(proposedKey);
-
-    shardkeyutil::validateShardKeyIndexExistsOrCreateIfPossible(
-        opCtx,
-        nss,
-        proposedKey,
-        shardKeyPattern,
-        *request.getCollation(),
-        request.getUnique(),
-        shardkeyutil::ValidationBehaviorsShardCollection(opCtx));
-
-    // Wait until the index is majority written, to prevent having the collection commited to the
-    // config server, but the index creation rolled backed on stepdowns.
-    WriteConcernResult ignoreResult;
-    auto latestOpTime = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp();
-    uassertStatusOK(waitForWriteConcern(
-        opCtx, latestOpTime, ShardingCatalogClient::kMajorityWriteConcern, &ignoreResult));
-
-    auto tags = getTagsAndValidate(opCtx, nss, proposedKey, shardKeyPattern);
-    auto uuid = getOrGenerateUUID(opCtx, nss, request);
+    auto tags = getTagsAndValidate(opCtx, nss, request.getKey());
+    auto uuid =
+        request.getGetUUIDfromPrimaryShard() ? *getUUIDFromPrimaryShard(opCtx, nss) : UUID::gen();
 
     const bool isEmpty = checkIfCollectionIsEmpty(opCtx, nss);
-    return {uuid, std::move(shardKeyPattern), tags, isEmpty};
+    return {uuid, ShardKeyPattern(request.getKey()), tags, isEmpty};
 }
 
 void logStartShardCollection(OperationContext* opCtx,
@@ -640,9 +599,24 @@ CreateCollectionResponse shardCollection(OperationContext* opCtx,
 
         // Fail if there are partially written chunks from a previous failed shardCollection.
         checkForExistingChunks(opCtx, nss);
-
         checkCollation(opCtx, request);
 
+        // Create the collection locally
+        shardkeyutil::validateShardKeyIndexExistsOrCreateIfPossible(
+            opCtx,
+            nss,
+            ShardKeyPattern(request.getKey()),
+            *request.getCollation(),
+            request.getUnique(),
+            shardkeyutil::ValidationBehaviorsShardCollection(opCtx));
+
+        // Wait until the index is majority written, to prevent having the collection commited to
+        // the config server, but the index creation rolled backed on stepdowns.
+        WriteConcernResult ignoreResult;
+        auto latestOpTime = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp();
+        uassertStatusOK(waitForWriteConcern(
+            opCtx, latestOpTime, ShardingCatalogClient::kMajorityWriteConcern, &ignoreResult));
+
         targetState = calculateTargetState(opCtx, nss, request);
 
         shardCollectionResponse.setCollectionUUID(targetState->uuid);
diff --git a/src/mongo/db/s/shard_key_util.cpp b/src/mongo/db/s/shard_key_util.cpp
index 8bb381910b9..4a812cf3055 100644
--- a/src/mongo/db/s/shard_key_util.cpp
+++ b/src/mongo/db/s/shard_key_util.cpp
@@ -103,7 +103,6 @@ BSONObj makeCreateIndexesCmd(const NamespaceString& nss,
 
 void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
                                                    const NamespaceString& nss,
-                                                   const BSONObj& proposedKey,
                                                    const ShardKeyPattern& shardKeyPattern,
                                                    const boost::optional<BSONObj>& defaultCollation,
                                                    bool unique,
@@ -116,7 +115,8 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
         bool isUnique = idx["unique"].trueValue();
         uassert(ErrorCodes::InvalidOptions,
                 str::stream() << "can't shard collection '" << nss.ns() << "' with unique index on "
-                              << currentKey << " and proposed shard key " << proposedKey
+                              << currentKey << " and proposed shard key "
+                              << shardKeyPattern.toBSON()
                               << ". Uniqueness can't be maintained unless shard key is a prefix",
                 !isUnique || shardKeyPattern.isUniqueIndexCompatible(currentKey));
     }
@@ -127,14 +127,15 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
         BSONObj currentKey = idx["key"].embeddedObject();
         // Check 2.i. and 2.ii.
         if (!idx["sparse"].trueValue() && idx["filter"].eoo() && idx["collation"].eoo() &&
-            proposedKey.isPrefixOf(currentKey, SimpleBSONElementComparator::kInstance)) {
+            shardKeyPattern.toBSON().isPrefixOf(currentKey,
+                                                SimpleBSONElementComparator::kInstance)) {
             // We can't currently use hashed indexes with a non-default hash seed
             // Check iv.
             // Note that this means that, for sharding, we only support one hashed index
             // per field per collection.
             uassert(ErrorCodes::InvalidOptions,
                     str::stream() << "can't shard collection " << nss.ns()
-                                  << " with hashed shard key " << proposedKey
+                                  << " with hashed shard key " << shardKeyPattern.toBSON()
                                   << " because the hashed index uses a non-default seed of "
                                   << idx["seed"].numberInt(),
                     !shardKeyPattern.isHashedPattern() || idx["seed"].eoo() ||
@@ -145,12 +146,12 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
 
     // 3. If proposed key is required to be unique, additionally check for exact match.
     if (hasUsefulIndexForKey && unique) {
-        BSONObj eqQuery = BSON("ns" << nss.ns() << "key" << proposedKey);
+        BSONObj eqQuery = BSON("ns" << nss.ns() << "key" << shardKeyPattern.toBSON());
         BSONObj eqQueryResult;
 
         for (const auto& idx : indexes) {
             if (SimpleBSONObjComparator::kInstance.evaluate(idx["key"].embeddedObject() ==
-                                                            proposedKey)) {
+                                                            shardKeyPattern.toBSON())) {
                 eqQueryResult = idx;
                 break;
             }
@@ -164,7 +165,8 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
             BSONObj currKey = eqQueryResult["key"].embeddedObject();
             bool isCurrentID = (currKey.firstElementFieldNameStringData() == "_id");
             uassert(ErrorCodes::InvalidOptions,
-                    str::stream() << "can't shard collection " << nss.ns() << ", " << proposedKey
+                    str::stream() << "can't shard collection " << nss.ns() << ", "
+                                  << shardKeyPattern.toBSON()
                                   << " index not unique, and unique index explicitly specified",
                     isExplicitlyUnique || isCurrentID);
         }
@@ -172,7 +174,7 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
 
     if (hasUsefulIndexForKey) {
         // Check 2.iii Make sure that there is a useful, non-multikey index available.
-        behaviors.verifyUsefulNonMultiKeyIndex(nss, proposedKey);
+        behaviors.verifyUsefulNonMultiKeyIndex(nss, shardKeyPattern.toBSON());
         return;
     }
 
@@ -183,7 +185,7 @@ void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
     //    to call ensureIndex on primary shard, since indexes get copied to receiving shard
     //    whenever a migrate occurs. If the collection has a default collation, explicitly send
     //    the simple collation as part of the createIndex request.
-    behaviors.createShardKeyIndex(nss, proposedKey, defaultCollation, unique);
+    behaviors.createShardKeyIndex(nss, shardKeyPattern.toBSON(), defaultCollation, unique);
 }
 
 std::vector<BSONObj> ValidationBehaviorsShardCollection::loadIndexes(
diff --git a/src/mongo/db/s/shard_key_util.h b/src/mongo/db/s/shard_key_util.h
index e5ab23683eb..619ced60915 100644
--- a/src/mongo/db/s/shard_key_util.h
+++ b/src/mongo/db/s/shard_key_util.h
@@ -145,7 +145,6 @@ private:
  */
 void validateShardKeyIndexExistsOrCreateIfPossible(OperationContext* opCtx,
                                                    const NamespaceString& nss,
-                                                   const BSONObj& proposedKey,
                                                    const ShardKeyPattern& shardKeyPattern,
                                                    const boost::optional<BSONObj>& defaultCollation,
                                                    bool unique,
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index 1c1eb0f4c9e..96c4e4851d0 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -59,7 +59,6 @@ env.Library(
         '$BUILD_DIR/mongo/db/session_catalog',
         '$BUILD_DIR/mongo/db/shared_request_handling',
         'async_requests_sender',
-        'common_s',
         'grid',
     ],
     LIBDEPS_PRIVATE=[
@@ -97,15 +96,14 @@ env.Library(
         'client/sharding_network_connection_hook.cpp',
     ],
     LIBDEPS=[
-        '$BUILD_DIR/mongo/executor/async_multicaster',
-        '$BUILD_DIR/mongo/s/catalog/sharding_catalog_client_impl',
         '$BUILD_DIR/mongo/util/periodic_runner_factory',
-        'common_s',
     ],
     LIBDEPS_PRIVATE=[
+        '$BUILD_DIR/mongo/executor/async_multicaster',
         '$BUILD_DIR/mongo/executor/connection_pool_executor',
         '$BUILD_DIR/mongo/executor/thread_pool_task_executor',
-        "$BUILD_DIR/mongo/idl/server_parameter",
+        '$BUILD_DIR/mongo/idl/server_parameter',
+        'catalog/sharding_catalog_client_impl',
         'coreshard',
         'sharding_task_executor',
     ],
@@ -515,7 +513,6 @@ env.Library(
         'cluster_last_error_info',
         'commands/cluster_commands',
         'committed_optime_metadata_hook',
-        'common_s',
         'mongos_initializers',
         'mongos_topology_coordinator',
         'query/cluster_cursor_cleanup_job',
@@ -636,7 +633,6 @@ env.CppUnitTest(
         'catalog/sharding_catalog_client_mock',
         'chunk_writes_tracker',
         'cluster_last_error_info',
-        'common_s',
         'coreshard',
         'mongos_topology_coordinator',
         'sessions_collection_sharded',
diff --git a/src/mongo/s/client/SConscript b/src/mongo/s/client/SConscript
index 71ebe87178f..b8cffeb357b 100644
--- a/src/mongo/s/client/SConscript
+++ b/src/mongo/s/client/SConscript
@@ -33,7 +33,6 @@ env.Library(
     LIBDEPS=[
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/client/remote_command_retry_scheduler',
-        '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/s/write_ops/batch_write_types',
     ],
 )