SERVER-41951 Make the findAndModify path on mongos attach databaseVersion when sending shardVersion for Unsharded collections.

5 files changed, 30 insertions, 16 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
index f8a1525d542..2f323b8a32c 100644
--- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
+++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
@@ -143,6 +143,7 @@ selector:
   - jstests/sharding/move_primary_clone_test.js
   - jstests/sharding/database_versioning_safe_secondary_reads.js
   - jstests/sharding/clone_catalog_data.js
+  - jstests/sharding/database_and_shard_versioning_all_commands.js
   # Enable if SERVER-41813 is backported or 4.4 becomes last-stable
   - jstests/sharding/invalid_system_views_sharded_collection.js
 
diff --git a/jstests/sharding/database_and_shard_versioning_all_commands.js b/jstests/sharding/database_and_shard_versioning_all_commands.js
index 6927a756252..21562c8c56c 100644
--- a/jstests/sharding/database_and_shard_versioning_all_commands.js
+++ b/jstests/sharding/database_and_shard_versioning_all_commands.js
@@ -213,7 +213,7 @@
             command: {find: collName, filter: {x: 1}},
         },
         findAndModify: {
-            sendsDbVersion: false,
+            sendsDbVersion: true,
             sendsShardVersion: true,
             command: {findAndModify: collName, query: {_id: 0}, remove: true}
         },
diff --git a/src/mongo/s/cluster_commands_helpers.cpp b/src/mongo/s/cluster_commands_helpers.cpp
index 1cfe08eed3b..bd3c4d8b323 100644
--- a/src/mongo/s/cluster_commands_helpers.cpp
+++ b/src/mongo/s/cluster_commands_helpers.cpp
@@ -102,15 +102,7 @@ std::unique_ptr<WriteConcernErrorDetail> getWriteConcernErrorDetailFromBSONObj(c
 namespace {
 
 BSONObj appendDbVersionIfPresent(BSONObj cmdObj, const CachedDatabaseInfo& dbInfo) {
-    // Attach the databaseVersion if we have one cached for the database.
-    auto dbVersion = dbInfo.databaseVersion();
-    if (databaseVersion::isFixed(dbVersion)) {
-        return cmdObj;
-    }
-
-    BSONObjBuilder cmdWithVersionBob(std::move(cmdObj));
-    cmdWithVersionBob.append("databaseVersion", dbVersion.toBSON());
-    return cmdWithVersionBob.obj();
+    return appendDbVersionIfPresent(std::move(cmdObj), dbInfo.databaseVersion());
 }
 
 const auto kAllowImplicitCollectionCreation = "allowImplicitCollectionCreation"_sd;
@@ -254,6 +246,15 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
     return responses;
 }
 
+BSONObj appendDbVersionIfPresent(BSONObj cmdObj, DatabaseVersion dbVersion) {
+    if (databaseVersion::isFixed(dbVersion)) {
+        return cmdObj;
+    }
+    BSONObjBuilder cmdWithDbVersion(std::move(cmdObj));
+    cmdWithDbVersion.append("databaseVersion", dbVersion.toBSON());
+    return cmdWithDbVersion.obj();
+}
+
 BSONObj appendShardVersion(BSONObj cmdObj, ChunkVersion version) {
     BSONObjBuilder cmdWithVersionBob(std::move(cmdObj));
     version.appendToCommand(&cmdWithVersionBob);
diff --git a/src/mongo/s/cluster_commands_helpers.h b/src/mongo/s/cluster_commands_helpers.h
index ae3b89b842e..9be10a0f481 100644
--- a/src/mongo/s/cluster_commands_helpers.h
+++ b/src/mongo/s/cluster_commands_helpers.h
@@ -71,6 +71,11 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
     const std::set<ErrorCodes::Error>& ignorableErrors = {});
 
 /**
+ * Returns a copy of 'cmdObj' with 'databaseVersion' appended.
+ */
+BSONObj appendDbVersionIfPresent(BSONObj cmdObj, DatabaseVersion dbVersion);
+
+/**
  * Returns a copy of 'cmdObj' with 'version' appended.
  */
 BSONObj appendShardVersion(BSONObj cmdObj, ChunkVersion version);
diff --git a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
index 1f70c6080a0..addeb0d4eb6 100644
--- a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
@@ -202,6 +202,7 @@ public:
         _runCommand(opCtx,
                     shard->getId(),
                     (chunkMgr ? chunkMgr->getVersion(shard->getId()) : ChunkVersion::UNSHARDED()),
+                    boost::none,
                     nss,
                     explainCmd,
                     &bob);
@@ -238,6 +239,7 @@ public:
             _runCommand(opCtx,
                         routingInfo.db().primaryId(),
                         ChunkVersion::UNSHARDED(),
+                        routingInfo.db().databaseVersion(),
                         nss,
                         cmdObjForShard,
                         &result);
@@ -254,6 +256,7 @@ public:
         _runCommand(opCtx,
                     chunk.getShardId(),
                     chunkMgr->getVersion(chunk.getShardId()),
+                    boost::none,
                     nss,
                     cmdObjForShard,
                     &result);
@@ -265,16 +268,19 @@ private:
     static void _runCommand(OperationContext* opCtx,
                             const ShardId& shardId,
                             const ChunkVersion& shardVersion,
+                            boost::optional<DatabaseVersion> dbVersion,
                             const NamespaceString& nss,
                             const BSONObj& cmdObj,
                             BSONObjBuilder* result) {
         bool isRetryableWrite = opCtx->getTxnNumber() && !TransactionRouter::get(opCtx);
         const auto response = [&] {
             std::vector<AsyncRequestsSender::Request> requests;
-            requests.emplace_back(
-                shardId,
-                appendShardVersion(CommandHelpers::filterCommandRequestForPassthrough(cmdObj),
-                                   shardVersion));
+            BSONObj filteredCmdObj = CommandHelpers::filterCommandRequestForPassthrough(cmdObj);
+            BSONObj cmdObjWithVersions(std::move(filteredCmdObj));
+            if (dbVersion) {
+                cmdObjWithVersions = appendDbVersionIfPresent(cmdObjWithVersions, *dbVersion);
+            }
+            requests.emplace_back(shardId, appendShardVersion(cmdObjWithVersions, shardVersion));
 
             MultiStatementTransactionRequestsSender ars(
                 opCtx,
@@ -294,7 +300,8 @@ private:
 
         const auto responseStatus = getStatusFromCommandResult(response.data);
         if (ErrorCodes::isNeedRetargettingError(responseStatus.code()) ||
-            ErrorCodes::isSnapshotError(responseStatus.code())) {
+            ErrorCodes::isSnapshotError(responseStatus.code()) ||
+            responseStatus.code() == ErrorCodes::StaleDbVersion) {
             // Command code traps this exception and re-runs
             uassertStatusOK(responseStatus.withContext("findAndModify"));
         }
@@ -312,7 +319,7 @@ private:
                     // since it will be run as a transaction it will take the other code path to
                     // updateShardKeyValueOnWouldChangeOwningShardError.
                     documentShardKeyUpdateUtil::startTransactionForShardKeyUpdate(opCtx);
-                    _runCommand(opCtx, shardId, shardVersion, nss, cmdObj, result);
+                    _runCommand(opCtx, shardId, shardVersion, dbVersion, nss, cmdObj, result);
                     auto commitResponse =
                         documentShardKeyUpdateUtil::commitShardKeyUpdateTransaction(opCtx);