SERVER-48653 Return updated ShardVersion in _configsvrCommitChunkSplit to avoid blind metadata refresh (BACKPORT-9050)

7 files changed, 50 insertions, 34 deletions

diff --git a/src/mongo/db/s/chunk_splitter.cpp b/src/mongo/db/s/chunk_splitter.cpp
index 2679e99f02a..bd4c8878544 100644
--- a/src/mongo/db/s/chunk_splitter.cpp
+++ b/src/mongo/db/s/chunk_splitter.cpp
@@ -92,15 +92,15 @@ Status splitChunkAtMultiplePoints(OperationContext* opCtx,
                               << " parts at a time."};
     }
 
-    const auto status = splitChunk(opCtx,
-                                   nss,
-                                   shardKeyPattern.toBSON(),
-                                   chunkRange,
-                                   splitPoints,
-                                   shardId.toString(),
-                                   collectionVersion.epoch());
-
-    return status.getStatus().withContext("split failed");
+    return splitChunk(opCtx,
+                      nss,
+                      shardKeyPattern.toBSON(),
+                      chunkRange,
+                      splitPoints,
+                      shardId.toString(),
+                      collectionVersion.epoch())
+        .getStatus()
+        .withContext("split failed");
 }
 
 /**
diff --git a/src/mongo/db/s/config/configsvr_split_chunk_command.cpp b/src/mongo/db/s/config/configsvr_split_chunk_command.cpp
index b2d3f700ac8..39c4e43daad 100644
--- a/src/mongo/db/s/config/configsvr_split_chunk_command.cpp
+++ b/src/mongo/db/s/config/configsvr_split_chunk_command.cpp
@@ -114,14 +114,14 @@ public:
 
         auto parsedRequest = uassertStatusOK(SplitChunkRequest::parseFromConfigCommand(cmdObj));
 
-        Status splitChunkResult =
+        auto shardVers = uassertStatusOK(
             ShardingCatalogManager::get(opCtx)->commitChunkSplit(opCtx,
                                                                  parsedRequest.getNamespace(),
                                                                  parsedRequest.getEpoch(),
                                                                  parsedRequest.getChunkRange(),
                                                                  parsedRequest.getSplitPoints(),
-                                                                 parsedRequest.getShardName());
-        uassertStatusOK(splitChunkResult);
+                                                                 parsedRequest.getShardName()));
+        result.appendElements(shardVers);
 
         return true;
     }
diff --git a/src/mongo/db/s/config/sharding_catalog_manager.h b/src/mongo/db/s/config/sharding_catalog_manager.h
index 417f14d5a80..485e2923f86 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager.h
+++ b/src/mongo/db/s/config/sharding_catalog_manager.h
@@ -180,13 +180,16 @@ public:
     /**
      * Updates metadata in the config.chunks collection to show the given chunk as split into
      * smaller chunks at the specified split points.
-     */
-    Status commitChunkSplit(OperationContext* opCtx,
-                            const NamespaceString& nss,
-                            const OID& requestEpoch,
-                            const ChunkRange& range,
-                            const std::vector<BSONObj>& splitPoints,
-                            const std::string& shardName);
+     *
+     * Returns a BSON object with the newly produced chunk version after the migration:
+     *   - shardVersion - The new shard version of the source shard
+     */
+    StatusWith<BSONObj> commitChunkSplit(OperationContext* opCtx,
+                                         const NamespaceString& nss,
+                                         const OID& requestEpoch,
+                                         const ChunkRange& range,
+                                         const std::vector<BSONObj>& splitPoints,
+                                         const std::string& shardName);
 
     /**
      * Updates metadata in the config.chunks collection so the chunks with given boundaries are seen
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
index 3072cd94c9c..0eabb6af267 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp
@@ -298,12 +298,13 @@ StatusWith<ChunkVersion> getMaxChunkVersionFromQueryResponse(
 
 }  // namespace
 
-Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx,
-                                                const NamespaceString& nss,
-                                                const OID& requestEpoch,
-                                                const ChunkRange& range,
-                                                const std::vector<BSONObj>& splitPoints,
-                                                const std::string& shardName) {
+StatusWith<BSONObj> ShardingCatalogManager::commitChunkSplit(
+    OperationContext* opCtx,
+    const NamespaceString& nss,
+    const OID& requestEpoch,
+    const ChunkRange& range,
+    const std::vector<BSONObj>& splitPoints,
+    const std::string& shardName) {
     // Take _kChunkOpLock in exclusive mode to prevent concurrent chunk splits, merges, and
     // migrations
     // TODO(SERVER-25359): Replace with a collection-specific lock map to allow splits/merges/
@@ -546,7 +547,9 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx,
         }
     }
 
-    return applyOpsStatus;
+    BSONObjBuilder result;
+    currentMaxVersion.appendToCommand(&result);
+    return result.obj();
 }
 
 Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx,
diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
index fbcc31a0557..8a3b1026e7d 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
@@ -78,8 +78,7 @@ void onShardVersionMismatch(OperationContext* opCtx,
     }();
 
     if (currentShardVersion) {
-        if (currentShardVersion->epoch() == shardVersionReceived.epoch() &&
-            currentShardVersion->majorVersion() >= shardVersionReceived.majorVersion()) {
+        if (shardVersionReceived.isOlderThan(*currentShardVersion)) {
             // Don't need to remotely reload if we're in the same epoch and the requested version is
             // smaller than the one we know about. This means that the remote side is behind.
             return;
diff --git a/src/mongo/db/s/split_chunk.cpp b/src/mongo/db/s/split_chunk.cpp
index 031513c9162..89cddf3d812 100644
--- a/src/mongo/db/s/split_chunk.cpp
+++ b/src/mongo/db/s/split_chunk.cpp
@@ -192,9 +192,22 @@ StatusWith<boost::optional<ChunkRange>> splitChunk(OperationContext* opCtx,
         return cmdResponseStatus.getStatus();
     }
 
+    // old versions might not have the shardVersion field
+    const Shard::CommandResponse& cmdResponse = cmdResponseStatus.getValue();
+    if (cmdResponse.response[ChunkVersion::kShardVersionField]) {
+        const auto cv = uassertStatusOK(
+            ChunkVersion::parseWithField(cmdResponse.response, ChunkVersion::kShardVersionField));
+        uassertStatusOK(onShardVersionMismatchNoExcept(
+            opCtx, nss, std::move(cv), true /* forceRefreshFromThisThread */));
+    } else {
+        // Refresh metadata to pick up new chunk definitions (regardless of the results returned
+        // from running _configsvrCommitChunkMerge).
+        forceShardFilteringMetadataRefresh(opCtx, nss, true /* forceRefreshFromThisThread */);
+    }
+
     // Check commandStatus and writeConcernStatus
-    auto commandStatus = cmdResponseStatus.getValue().commandStatus;
-    auto writeConcernStatus = cmdResponseStatus.getValue().writeConcernStatus;
+    auto commandStatus = cmdResponse.commandStatus;
+    auto writeConcernStatus = cmdResponse.writeConcernStatus;
 
     // Send stale epoch if epoch of request did not match epoch of collection
     if (commandStatus == ErrorCodes::StaleEpoch) {
@@ -202,14 +215,12 @@ StatusWith<boost::optional<ChunkRange>> splitChunk(OperationContext* opCtx,
     }
 
     //
-    // If _configsvrCommitChunkSplit returned an error, refresh and look at the metadata to
+    // If _configsvrCommitChunkSplit returned an error, look at the metadata to
     // determine if the split actually did happen. This can happen if there's a network error
     // getting the response from the first call to _configsvrCommitChunkSplit, but it actually
     // succeeds, thus the automatic retry fails with a precondition violation, for example.
     //
     if (!commandStatus.isOK() || !writeConcernStatus.isOK()) {
-        forceShardFilteringMetadataRefresh(opCtx, nss);
-
         if (checkMetadataForSuccessfulSplitChunk(
                 opCtx, nss, expectedCollectionEpoch, chunkRange, splitKeys)) {
             // Split was committed.
@@ -253,7 +264,6 @@ StatusWith<boost::optional<ChunkRange>> splitChunk(OperationContext* opCtx,
                checkIfSingleDoc(opCtx, collection, idx, &frontChunk)) {
         return boost::optional<ChunkRange>(ChunkRange(frontChunk.getMin(), frontChunk.getMax()));
     }
-
     return boost::optional<ChunkRange>(boost::none);
 }
 
diff --git a/src/mongo/db/s/split_chunk.h b/src/mongo/db/s/split_chunk.h
index 748deb9e885..646ef3f461b 100644
--- a/src/mongo/db/s/split_chunk.h
+++ b/src/mongo/db/s/split_chunk.h
@@ -49,6 +49,7 @@ class StatusWith;
  * Attempts to split a chunk with the specified parameters. If the split fails, then the StatusWith
  * object returned will contain a Status with an ErrorCode regarding the cause of failure. If the
  * split succeeds, then the StatusWith object returned will contain Status::Ok().
+ * Will update the shard's filtering metadata.
  *
  * Additionally, splitChunk will attempt to perform top-chunk optimization. If top-chunk
  * optimization is performed, then the function will also return a ChunkRange, which contains the