From fc0c2123616158c1d2729761bec699ff03b72d57 Mon Sep 17 00:00:00 2001 From: Simon Gratzer Date: Tue, 11 May 2021 16:51:47 +0200 Subject: SERVER-48653 Return updated ShardVersion in _configsvrCommitChunkSplit to avoid blind metadata refresh (BACKPORT-9050) --- src/mongo/db/s/chunk_splitter.cpp | 18 +++++++++--------- .../db/s/config/configsvr_split_chunk_command.cpp | 6 +++--- src/mongo/db/s/config/sharding_catalog_manager.h | 17 ++++++++++------- .../sharding_catalog_manager_chunk_operations.cpp | 17 ++++++++++------- .../db/s/shard_filtering_metadata_refresh.cpp | 3 +-- src/mongo/db/s/split_chunk.cpp | 22 ++++++++++++++++------ src/mongo/db/s/split_chunk.h | 1 + 7 files changed, 50 insertions(+), 34 deletions(-) (limited to 'src/mongo/db') 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& 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 commitChunkSplit(OperationContext* opCtx, + const NamespaceString& nss, + const OID& requestEpoch, + const ChunkRange& range, + const std::vector& 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 getMaxChunkVersionFromQueryResponse( } // namespace -Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, - const NamespaceString& nss, - const OID& requestEpoch, - const ChunkRange& range, - const std::vector& splitPoints, - const std::string& shardName) { +StatusWith ShardingCatalogManager::commitChunkSplit( + OperationContext* opCtx, + const NamespaceString& nss, + const OID& requestEpoch, + const ChunkRange& range, + const std::vector& 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> 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> 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> splitChunk(OperationContext* opCtx, checkIfSingleDoc(opCtx, collection, idx, &frontChunk)) { return boost::optional(ChunkRange(frontChunk.getMin(), frontChunk.getMax())); } - return boost::optional(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 -- cgit v1.2.1