From 676f6aed12f9f56143223ab8c6a04439d02e94a8 Mon Sep 17 00:00:00 2001 From: Simon Graetzer Date: Thu, 17 Jun 2021 06:22:31 +0000 Subject: SERVER-48648 Return updated ShardVersion in _configsvrCommitChunkMerge to avoid blind metadata refresh (BACKPORT-9095) --- .../db/s/config/configsvr_merge_chunk_command.cpp | 6 +++--- src/mongo/db/s/config/sharding_catalog_manager.h | 15 +++++++++------ .../sharding_catalog_manager_chunk_operations.cpp | 18 +++++++++++------- src/mongo/db/s/merge_chunks_command.cpp | 14 +++++++++++--- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/mongo/db/s/config/configsvr_merge_chunk_command.cpp b/src/mongo/db/s/config/configsvr_merge_chunk_command.cpp index 117f20d6cb0..1078b740200 100644 --- a/src/mongo/db/s/config/configsvr_merge_chunk_command.cpp +++ b/src/mongo/db/s/config/configsvr_merge_chunk_command.cpp @@ -116,15 +116,15 @@ public: auto parsedRequest = uassertStatusOK(MergeChunkRequest::parseFromConfigCommand(cmdObj)); - Status mergeChunkResult = + const BSONObj shardVers = uassertStatusOK( ShardingCatalogManager::get(opCtx)->commitChunkMerge(opCtx, parsedRequest.getNamespace(), parsedRequest.getEpoch(), parsedRequest.getChunkBoundaries(), parsedRequest.getShardName(), - parsedRequest.getValidAfter()); + parsedRequest.getValidAfter())); + result.appendElements(shardVers); - uassertStatusOK(mergeChunkResult); return true; } } configsvrMergeChunkCmd; diff --git a/src/mongo/db/s/config/sharding_catalog_manager.h b/src/mongo/db/s/config/sharding_catalog_manager.h index 485e2923f86..de97df11fe4 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager.h +++ b/src/mongo/db/s/config/sharding_catalog_manager.h @@ -196,13 +196,16 @@ public: * merged into a single larger chunk. * If 'validAfter' is not set, this means the commit request came from an older server version, * which is not history-aware. + * + * Returns a BSON object with the newly produced chunk version after the merge: + * - shardVersion - The new shard version of the source shard */ - Status commitChunkMerge(OperationContext* opCtx, - const NamespaceString& nss, - const OID& requestEpoch, - const std::vector& chunkBoundaries, - const std::string& shardName, - const boost::optional& validAfter); + StatusWith commitChunkMerge(OperationContext* opCtx, + const NamespaceString& nss, + const OID& requestEpoch, + const std::vector& chunkBoundaries, + const std::string& shardName, + const boost::optional& validAfter); /** * Updates metadata in config.chunks collection to show the given chunk in its new shard. 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 0eabb6af267..699930b09b9 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 @@ -552,12 +552,13 @@ StatusWith ShardingCatalogManager::commitChunkSplit( return result.obj(); } -Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx, - const NamespaceString& nss, - const OID& requestEpoch, - const std::vector& chunkBoundaries, - const std::string& shardName, - const boost::optional& validAfter) { +StatusWith ShardingCatalogManager::commitChunkMerge( + OperationContext* opCtx, + const NamespaceString& nss, + const OID& requestEpoch, + const std::vector& chunkBoundaries, + const std::string& shardName, + const boost::optional& validAfter) { // This method must never be called with empty chunks to merge invariant(!chunkBoundaries.empty()); @@ -697,7 +698,10 @@ Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx, ->logChange(opCtx, "merge", nss.ns(), logDetail.obj(), WriteConcernOptions()) .ignore(); - return applyOpsStatus; + BSONObjBuilder result; + mergeVersion.appendToCommand(&result); + + return result.obj(); } StatusWith ShardingCatalogManager::commitChunkMigration( diff --git a/src/mongo/db/s/merge_chunks_command.cpp b/src/mongo/db/s/merge_chunks_command.cpp index 37cedd096eb..68bd7ab7b39 100644 --- a/src/mongo/db/s/merge_chunks_command.cpp +++ b/src/mongo/db/s/merge_chunks_command.cpp @@ -243,9 +243,17 @@ void mergeChunks(OperationContext* opCtx, configCmdObj, Shard::RetryPolicy::kIdempotent)); - // Refresh metadata to pick up new chunk definitions (regardless of the results returned from - // running _configsvrCommitChunkMerge). - forceShardFilteringMetadataRefresh(opCtx, nss, true /* forceRefreshFromThisThread */); + // old versions might not have the shardVersion field + 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 */); + } // If _configsvrCommitChunkMerge returned an error, look at this shard's metadata to determine // if the merge actually did happen. This can happen if there's a network error getting the -- cgit v1.2.1