diff options
37 files changed, 730 insertions, 480 deletions
diff --git a/src/mongo/db/pipeline/dispatch_shard_pipeline_test.cpp b/src/mongo/db/pipeline/dispatch_shard_pipeline_test.cpp index 0203dddb7a5..53896da0664 100644 --- a/src/mongo/db/pipeline/dispatch_shard_pipeline_test.cpp +++ b/src/mongo/db/pipeline/dispatch_shard_pipeline_test.cpp @@ -31,6 +31,7 @@ #include "mongo/db/pipeline/sharded_agg_helpers.h" #include "mongo/s/query/sharded_agg_test_fixture.h" #include "mongo/s/router.h" +#include "mongo/s/shard_version_factory.h" namespace mongo { namespace { @@ -196,11 +197,12 @@ TEST_F(DispatchShardPipelineTest, DispatchShardPipelineDoesNotRetryOnStaleConfig OID epoch{OID::gen()}; Timestamp timestamp{1, 0}; return createErrorCursorResponse( - {StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + {StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); future.default_timed_get(); @@ -247,11 +249,12 @@ TEST_F(DispatchShardPipelineTest, WrappedDispatchDoesRetryOnStaleConfigError) { // namespace, then mock out a successful response. onCommand([&](const executor::RemoteCommandRequest& request) { return createErrorCursorResponse( - {StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {2, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + {StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {2, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); diff --git a/src/mongo/db/pipeline/document_source_merge.cpp b/src/mongo/db/pipeline/document_source_merge.cpp index 3d045259cf2..b7b65c71084 100644 --- a/src/mongo/db/pipeline/document_source_merge.cpp +++ b/src/mongo/db/pipeline/document_source_merge.cpp @@ -484,12 +484,7 @@ boost::intrusive_ptr<DocumentSource> DocumentSourceMerge::createFromBson( auto fieldPaths = convertToFieldPaths(mergeSpec.getOn()); auto [mergeOnFields, targetCollectionVersion] = expCtx->mongoProcessInterface->ensureFieldsUniqueOrResolveDocumentKey( - expCtx, - std::move(fieldPaths), - mergeSpec.getTargetCollectionVersion() - ? boost::make_optional(mergeSpec.getTargetCollectionVersion()->placementVersion()) - : boost::none, - targetNss); + expCtx, std::move(fieldPaths), mergeSpec.getTargetCollectionVersion(), targetNss); return DocumentSourceMerge::create(std::move(targetNss), expCtx, @@ -558,11 +553,7 @@ Value DocumentSourceMerge::serialize(boost::optional<ExplainOptions::Verbosity> } return mergeOnFields; }()); - spec.setTargetCollectionVersion( - _targetCollectionVersion - ? boost::make_optional(ShardVersion(*_targetCollectionVersion, - boost::optional<CollectionIndexes>(boost::none))) - : boost::none); + spec.setTargetCollectionVersion(_targetCollectionVersion); return Value(Document{{getSourceName(), spec.toBSON()}}); } diff --git a/src/mongo/db/pipeline/document_source_merge.idl b/src/mongo/db/pipeline/document_source_merge.idl index 1dcd2168818..263dd509274 100644 --- a/src/mongo/db/pipeline/document_source_merge.idl +++ b/src/mongo/db/pipeline/document_source_merge.idl @@ -33,12 +33,12 @@ global: cpp_includes: - "mongo/db/namespace_string.h" - "mongo/db/pipeline/document_source_merge_spec.h" - - "mongo/s/shard_version.h" + - "mongo/s/chunk_version.h" imports: - "mongo/db/pipeline/document_source_merge_modes.idl" - "mongo/db/basic_types.idl" - - "mongo/s/sharding_types.idl" + - "mongo/s/chunk_version.idl" types: MergeTargetNss: bson_serialization_type: any @@ -113,9 +113,9 @@ structs: do not match. targetCollectionVersion: - type: shard_version + type: ChunkVersion optional: true - description: If set, the collection's ShardVersion found when parsed on mongos. Can + description: If set, the collection's ChunkVersion found when parsed on mongos. Can be used to check if a collection has since been dropped and re-created, in which case the shard key may have changed, or had its shard key refined. This also can be used to detect if the collection has gone diff --git a/src/mongo/db/pipeline/process_interface/shardsvr_process_interface.cpp b/src/mongo/db/pipeline/process_interface/shardsvr_process_interface.cpp index 952de4b1116..594bd87a060 100644 --- a/src/mongo/db/pipeline/process_interface/shardsvr_process_interface.cpp +++ b/src/mongo/db/pipeline/process_interface/shardsvr_process_interface.cpp @@ -51,6 +51,7 @@ #include "mongo/s/cluster_write.h" #include "mongo/s/query/document_source_merge_cursors.h" #include "mongo/s/router.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/stale_shard_version_helpers.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kQuery @@ -78,11 +79,11 @@ void ShardServerProcessInterface::checkRoutingInfoEpochOrThrow( // Mark the cache entry routingInfo for the 'nss' and 'shardId' if the entry is staler than // 'targetCollectionVersion'. - const ShardVersion ignoreIndexVersion{ + const ShardVersion ignoreIndexVersion = ShardVersionFactory::make( targetCollectionVersion, currentGlobalIndexesInfo ? boost::make_optional(currentGlobalIndexesInfo->getCollectionIndexes()) - : boost::none}; + : boost::none); catalogCache->invalidateShardOrEntireCollectionEntryForShardedCollection( nss, ignoreIndexVersion, shardId); diff --git a/src/mongo/db/pipeline/sharded_union_test.cpp b/src/mongo/db/pipeline/sharded_union_test.cpp index d7c82e12075..e29c8d19604 100644 --- a/src/mongo/db/pipeline/sharded_union_test.cpp +++ b/src/mongo/db/pipeline/sharded_union_test.cpp @@ -36,6 +36,7 @@ #include "mongo/db/repl/read_concern_args.h" #include "mongo/db/views/resolved_view.h" #include "mongo/s/query/sharded_agg_test_fixture.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/stale_exception.h" #include "mongo/unittest/unittest.h" @@ -162,11 +163,12 @@ TEST_F(ShardedUnionTest, RetriesSubPipelineOnStaleConfigError) { OID epoch{OID::gen()}; Timestamp timestamp{1, 0}; return createErrorCursorResponse( - Status{StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + Status{StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); @@ -251,11 +253,12 @@ TEST_F(ShardedUnionTest, CorrectlySplitsSubPipelineIfRefreshedDistributionRequir OID epoch{OID::gen()}; Timestamp timestamp{1, 0}; return createErrorCursorResponse( - Status{StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + Status{StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); @@ -348,20 +351,22 @@ TEST_F(ShardedUnionTest, AvoidsSplittingSubPipelineIfRefreshedDistributionDoesNo onCommand([&](const executor::RemoteCommandRequest& request) { return createErrorCursorResponse( - Status{StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + Status{StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); onCommand([&](const executor::RemoteCommandRequest& request) { return createErrorCursorResponse( - Status{StaleConfigInfo(kTestAggregateNss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"0"}), + Status{StaleConfigInfo( + kTestAggregateNss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"0"}), "Mock error: shard version mismatch"}); }); diff --git a/src/mongo/db/s/balancer/balancer_commands_scheduler_test.cpp b/src/mongo/db/s/balancer/balancer_commands_scheduler_test.cpp index 64f8debc967..8ad8f15cb64 100644 --- a/src/mongo/db/s/balancer/balancer_commands_scheduler_test.cpp +++ b/src/mongo/db/s/balancer/balancer_commands_scheduler_test.cpp @@ -33,7 +33,7 @@ #include "mongo/db/s/config/config_server_test_fixture.h" #include "mongo/s/catalog/sharding_catalog_client_mock.h" #include "mongo/s/request_types/move_range_request_gen.h" - +#include "mongo/s/shard_version_factory.h" namespace mongo { namespace { @@ -317,7 +317,8 @@ TEST_F(BalancerCommandsSchedulerTest, SuccessfulRequestChunkDataSizeCommand) { kNss, chunk.getShard(), chunk.getRange(), - ShardVersion(chunk.getVersion(), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(chunk.getVersion(), + boost::optional<CollectionIndexes>(boost::none)), KeyPattern(BSON("x" << 1)), false /* issuedByRemoteUser */, (kDefaultMaxChunkSizeBytes / 100) * 25 /* maxSize */); diff --git a/src/mongo/db/s/chunk_operation_precondition_checks.cpp b/src/mongo/db/s/chunk_operation_precondition_checks.cpp index ada542db76e..b36ac22aacc 100644 --- a/src/mongo/db/s/chunk_operation_precondition_checks.cpp +++ b/src/mongo/db/s/chunk_operation_precondition_checks.cpp @@ -31,6 +31,7 @@ #include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/s/sharding_state.h" +#include "mongo/s/shard_version_factory.h" namespace mongo { @@ -69,7 +70,7 @@ CollectionPlacementAndIndexInfo checkCollectionIdentity( const auto placementVersion = metadata.getShardVersion(); const auto shardVersion = - ShardVersion(placementVersion, scopedCsr->getCollectionIndexes(opCtx)); + ShardVersionFactory::make(metadata, scopedCsr->getCollectionIndexes(opCtx)); uassert(StaleConfigInfo(nss, ShardVersion::IGNORED() /* receivedVersion */, @@ -98,10 +99,10 @@ void checkShardKeyPattern(OperationContext* opCtx, const ChunkRange& chunkRange) { const auto shardId = ShardingState::get(opCtx)->shardId(); const auto& keyPattern = metadata.getKeyPattern(); - const auto shardVersion = - ShardVersion(metadata.getShardVersion(), - globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) - : boost::none); + const auto shardVersion = ShardVersionFactory::make( + metadata, + globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) + : boost::none); uassert(StaleConfigInfo(nss, ShardVersion::IGNORED() /* receivedVersion */, @@ -119,10 +120,10 @@ void checkChunkMatchesRange(OperationContext* opCtx, const boost::optional<GlobalIndexesCache>& globalIndexInfo, const ChunkRange& chunkRange) { const auto shardId = ShardingState::get(opCtx)->shardId(); - const auto shardVersion = - ShardVersion(metadata.getShardVersion(), - globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) - : boost::none); + const auto shardVersion = ShardVersionFactory::make( + metadata, + globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) + : boost::none); ChunkType existingChunk; uassert(StaleConfigInfo(nss, @@ -148,10 +149,10 @@ void checkRangeWithinChunk(OperationContext* opCtx, const boost::optional<GlobalIndexesCache>& globalIndexInfo, const ChunkRange& chunkRange) { const auto shardId = ShardingState::get(opCtx)->shardId(); - const auto shardVersion = - ShardVersion(metadata.getShardVersion(), - globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) - : boost::none); + const auto shardVersion = ShardVersionFactory::make( + metadata, + globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) + : boost::none); ChunkType existingChunk; uassert(StaleConfigInfo(nss, @@ -170,10 +171,10 @@ void checkRangeOwnership(OperationContext* opCtx, const boost::optional<GlobalIndexesCache>& globalIndexInfo, const ChunkRange& chunkRange) { const auto shardId = ShardingState::get(opCtx)->shardId(); - const auto shardVersion = - ShardVersion(metadata.getShardVersion(), - globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) - : boost::none); + const auto shardVersion = ShardVersionFactory::make( + metadata, + globalIndexInfo ? boost::make_optional(globalIndexInfo->getCollectionIndexes()) + : boost::none); ChunkType existingChunk; BSONObj minKey = chunkRange.getMin(); diff --git a/src/mongo/db/s/collection_metadata_filtering_test.cpp b/src/mongo/db/s/collection_metadata_filtering_test.cpp index 017cc3362c1..771904eb229 100644 --- a/src/mongo/db/s/collection_metadata_filtering_test.cpp +++ b/src/mongo/db/s/collection_metadata_filtering_test.cpp @@ -32,6 +32,7 @@ #include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/s/shard_server_test_fixture.h" #include "mongo/s/catalog/type_chunk.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/type_collection_common_types_gen.h" namespace mongo { @@ -157,8 +158,8 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsInTheFuture) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto scopedCss = CollectionShardingState::assertCollectionLockedAndAcquire(operationContext(), kNss); @@ -188,8 +189,8 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsInThePast) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto scopedCss = CollectionShardingState::assertCollectionLockedAndAcquire(operationContext(), kNss); @@ -227,8 +228,8 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsTooFarInThePastThrowsStal ScopedSetShardRole scopedSetShardRole{ operationContext(), kNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto scopedCss = CollectionShardingState::assertCollectionLockedAndAcquire(operationContext(), kNss); diff --git a/src/mongo/db/s/collection_sharding_runtime.cpp b/src/mongo/db/s/collection_sharding_runtime.cpp index f007c17c777..a8a28caaa6a 100644 --- a/src/mongo/db/s/collection_sharding_runtime.cpp +++ b/src/mongo/db/s/collection_sharding_runtime.cpp @@ -41,6 +41,7 @@ #include "mongo/db/s/sharding_state.h" #include "mongo/logv2/log.h" #include "mongo/s/grid.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_feature_flags_gen.h" #include "mongo/util/duration.h" @@ -484,7 +485,8 @@ CollectionShardingRuntime::_getMetadataWithVersionCheckAt( const auto wantedIndexVersion = wantedCollectionIndexes ? boost::make_optional(wantedCollectionIndexes->indexVersion()) : boost::none; - const auto wantedShardVersion = ShardVersion(wantedPlacementVersion, wantedCollectionIndexes); + const auto wantedShardVersion = + ShardVersionFactory::make(currentMetadata, wantedCollectionIndexes); const ChunkVersion receivedPlacementVersion = receivedShardVersion.placementVersion(); const boost::optional<Timestamp> receivedIndexVersion = receivedShardVersion.indexVersion(); diff --git a/src/mongo/db/s/collection_sharding_runtime_test.cpp b/src/mongo/db/s/collection_sharding_runtime_test.cpp index 76443bdc7eb..2473a016548 100644 --- a/src/mongo/db/s/collection_sharding_runtime_test.cpp +++ b/src/mongo/db/s/collection_sharding_runtime_test.cpp @@ -46,6 +46,7 @@ #include "mongo/db/vector_clock.h" #include "mongo/s/catalog/sharding_catalog_client_mock.h" #include "mongo/s/catalog_cache_loader_mock.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/stdx/chrono.h" #include "mongo/stdx/thread.h" #include "mongo/util/fail_point.h" @@ -102,7 +103,7 @@ TEST_F(CollectionShardingRuntimeTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kTestNss, - ShardVersion(metadata.getShardVersion(), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(metadata, boost::optional<CollectionIndexes>(boost::none)), boost::none /* databaseVersion */}; ASSERT_THROWS_CODE(csr.getCollectionDescription(opCtx), DBException, ErrorCodes::StaleConfig); } @@ -124,7 +125,7 @@ TEST_F(CollectionShardingRuntimeTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kTestNss, - ShardVersion(metadata.getShardVersion(), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(metadata, boost::optional<CollectionIndexes>(boost::none)), boost::none /* databaseVersion */}; ASSERT_TRUE(csr.getCollectionDescription(opCtx).isSharded()); } @@ -191,7 +192,7 @@ TEST_F(CollectionShardingRuntimeTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kTestNss, - ShardVersion(metadata.getShardVersion(), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(metadata, boost::optional<CollectionIndexes>(boost::none)), boost::none /* databaseVersion */}; ASSERT_EQ(csr.getNumMetadataManagerChanges_forTest(), 1); @@ -237,9 +238,10 @@ TEST_F(CollectionShardingRuntimeTest, ReturnUnshardedMetadataInServerlessMode) { ScopedSetShardRole scopedSetShardRole2{ opCtx, NamespaceString::kLogicalSessionsNamespace, - ShardVersion(ChunkVersion(gen, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), /* shardVersion */ - boost::none /* databaseVersion */ + ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), /* shardVersion */ + boost::none /* databaseVersion */ }; CollectionShardingRuntime csrLogicalSession( diff --git a/src/mongo/db/s/create_collection_coordinator.cpp b/src/mongo/db/s/create_collection_coordinator.cpp index de449d4a88c..556544b8aad 100644 --- a/src/mongo/db/s/create_collection_coordinator.cpp +++ b/src/mongo/db/s/create_collection_coordinator.cpp @@ -62,6 +62,7 @@ #include "mongo/s/cluster_commands_helpers.h" #include "mongo/s/cluster_write.h" #include "mongo/s/grid.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_feature_flags_gen.h" @@ -1342,8 +1343,8 @@ void CreateCollectionCoordinator::_commit(OperationContext* opCtx, "numInitialChunks"_attr = _initialChunks->chunks.size(), "initialCollectionVersion"_attr = _initialChunks->collVersion()); - auto result = CreateCollectionResponse( - {placementVersion, boost::optional<CollectionIndexes>(boost::none)}); + auto result = CreateCollectionResponse(ShardVersionFactory::make( + placementVersion, boost::optional<CollectionIndexes>(boost::none))); result.setCollectionUUID(_collectionUUID); _result = std::move(result); diff --git a/src/mongo/db/s/op_observer_sharding_test.cpp b/src/mongo/db/s/op_observer_sharding_test.cpp index ab06005dd8b..618ca3603c4 100644 --- a/src/mongo/db/s/op_observer_sharding_test.cpp +++ b/src/mongo/db/s/op_observer_sharding_test.cpp @@ -41,6 +41,7 @@ #include "mongo/db/s/shard_server_test_fixture.h" #include "mongo/db/s/type_shard_identity.h" #include "mongo/db/session/session_catalog_mongod.h" +#include "mongo/s/shard_version_factory.h" namespace mongo { namespace { @@ -124,8 +125,8 @@ TEST_F(DocumentKeyStateTest, MakeDocumentKeyStateUnsharded) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kTestNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; AutoGetCollection autoColl(operationContext(), kTestNss, MODE_IX); @@ -150,8 +151,8 @@ TEST_F(DocumentKeyStateTest, MakeDocumentKeyStateShardedWithoutIdInShardKey) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kTestNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; AutoGetCollection autoColl(operationContext(), kTestNss, MODE_IX); @@ -179,8 +180,8 @@ TEST_F(DocumentKeyStateTest, MakeDocumentKeyStateShardedWithIdInShardKey) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kTestNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; AutoGetCollection autoColl(operationContext(), kTestNss, MODE_IX); @@ -208,8 +209,8 @@ TEST_F(DocumentKeyStateTest, MakeDocumentKeyStateShardedWithIdHashInShardKey) { ScopedSetShardRole scopedSetShardRole{ operationContext(), kTestNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; AutoGetCollection autoColl(operationContext(), kTestNss, MODE_IX); diff --git a/src/mongo/db/s/operation_sharding_state_test.cpp b/src/mongo/db/s/operation_sharding_state_test.cpp index 83553f1ed8c..7c94421d0fc 100644 --- a/src/mongo/db/s/operation_sharding_state_test.cpp +++ b/src/mongo/db/s/operation_sharding_state_test.cpp @@ -29,6 +29,7 @@ #include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/s/shard_server_test_fixture.h" +#include "mongo/s/shard_version_factory.h" namespace mongo { namespace { @@ -48,7 +49,8 @@ TEST_F(OperationShardingStateTest, ScopedSetShardRoleDbVersion) { TEST_F(OperationShardingStateTest, ScopedSetShardRoleShardVersion) { CollectionGeneration gen(OID::gen(), Timestamp(1, 0)); - ShardVersion shardVersion({gen, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion shardVersion = + ShardVersionFactory::make({gen, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); ScopedSetShardRole scopedSetShardRole(operationContext(), kNss, shardVersion, boost::none); auto& oss = OperationShardingState::get(operationContext()); @@ -60,14 +62,16 @@ TEST_F(OperationShardingStateTest, ScopedSetShardRoleChangeShardVersionSameNames { CollectionGeneration gen1(OID::gen(), Timestamp(10, 0)); - ShardVersion shardVersion1({gen1, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion shardVersion1 = ShardVersionFactory::make( + {gen1, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); ScopedSetShardRole scopedSetShardRole1( operationContext(), kNss, shardVersion1, boost::none); ASSERT_EQ(shardVersion1, *oss.getShardVersion(kNss)); } { CollectionGeneration gen2(OID::gen(), Timestamp(20, 0)); - ShardVersion shardVersion2({gen2, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion shardVersion2 = ShardVersionFactory::make( + {gen2, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); ScopedSetShardRole scopedSetShardRole2( operationContext(), kNss, shardVersion2, boost::none); ASSERT_EQ(shardVersion2, *oss.getShardVersion(kNss)); @@ -77,8 +81,10 @@ TEST_F(OperationShardingStateTest, ScopedSetShardRoleChangeShardVersionSameNames TEST_F(OperationShardingStateTest, ScopedSetShardRoleRecursiveShardVersionDifferentNamespaces) { CollectionGeneration gen1(OID::gen(), Timestamp(10, 0)); CollectionGeneration gen2(OID::gen(), Timestamp(20, 0)); - ShardVersion shardVersion1({gen1, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); - ShardVersion shardVersion2({gen2, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion shardVersion1 = + ShardVersionFactory::make({gen1, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion shardVersion2 = + ShardVersionFactory::make({gen2, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); ScopedSetShardRole scopedSetShardRole1(operationContext(), kNss, shardVersion1, boost::none); ScopedSetShardRole scopedSetShardRole2( @@ -115,7 +121,8 @@ TEST_F(OperationShardingStateTest, ScopedSetShardRoleAllowedShardVersionsWithFix // Any other shard version cannot be passed with a fixed dbVersion. DatabaseVersion dbv{DatabaseVersion::makeFixed()}; CollectionGeneration gen(OID::gen(), Timestamp(1, 0)); - ShardVersion sv({gen, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); + ShardVersion sv = ShardVersionFactory::make( + {gen, {1, 0}}, boost::optional<CollectionIndexes>(boost::none)); ASSERT_THROWS_CODE( [&] { ScopedSetShardRole scopedSetShardRole(operationContext(), kNss, sv, dbv); diff --git a/src/mongo/db/s/resharding/resharding_destined_recipient_test.cpp b/src/mongo/db/s/resharding/resharding_destined_recipient_test.cpp index 9b7e587ce3d..3ab26badfe5 100644 --- a/src/mongo/db/s/resharding/resharding_destined_recipient_test.cpp +++ b/src/mongo/db/s/resharding/resharding_destined_recipient_test.cpp @@ -49,6 +49,7 @@ #include "mongo/s/catalog_cache_loader_mock.h" #include "mongo/s/database_version.h" #include "mongo/s/shard_cannot_refresh_due_to_locks_held_exception.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/unittest/unittest.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kTest @@ -198,8 +199,8 @@ protected: ReshardingEnv env(CollectionCatalog::get(opCtx)->lookupUUIDByNSS(opCtx, kNss).value()); env.destShard = kShardList[1].getName(); CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - env.version = ShardVersion(ChunkVersion(gen, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)); + env.version = ShardVersionFactory::make(ChunkVersion(gen, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)); env.tempNss = NamespaceString::createNamespaceString_forTest( kNss.db(), fmt::format("{}{}", diff --git a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp index 6752df4100d..95abf9d2e86 100644 --- a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp @@ -39,6 +39,7 @@ #include "mongo/db/s/resharding/donor_document_gen.h" #include "mongo/db/s/resharding/resharding_donor_recipient_common.h" #include "mongo/db/s/shard_server_test_fixture.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/unittest/death_test.h" #include "mongo/util/fail_point.h" @@ -272,8 +273,8 @@ protected: ScopedSetShardRole scopedSetShardRole{ opCtx, sourceNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; CollectionShardingRuntime::assertCollectionLockedAndAcquireExclusive(opCtx, sourceNss) @@ -354,8 +355,8 @@ TEST_F(ReshardingDonorRecipientCommonInternalsTest, ConstructDonorDocumentFromRe ScopedSetShardRole scopedSetShardRole{ opCtx, kOriginalNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -375,8 +376,8 @@ TEST_F(ReshardingDonorRecipientCommonInternalsTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kTemporaryReshardingNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -395,8 +396,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, CreateDonorServiceInstance) { ScopedSetShardRole scopedSetShardRole{ opCtx, kOriginalNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -423,8 +424,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, CreateRecipientServiceInstance) { ScopedSetShardRole scopedSetShardRole{ opCtx, kTemporaryReshardingNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -452,8 +453,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kOriginalNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -475,8 +476,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ScopedSetShardRole scopedSetShardRole{ opCtx, kTemporaryReshardingNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -504,8 +505,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ProcessDonorFieldsWhenShardDoesntOwnA ScopedSetShardRole scopedSetShardRole{ opCtx, kOriginalNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -530,8 +531,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ProcessRecipientFieldsWhenShardDoesnt ScopedSetShardRole scopedSetShardRole{ opCtx, kTemporaryReshardingNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = @@ -557,8 +558,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ProcessReshardingFieldsWithoutDonorOr ScopedSetShardRole scopedSetShardRole{ opCtx, kTemporaryReshardingNss, - ShardVersion(metadata.getShardVersion(), - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + metadata, boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */}; auto reshardingFields = diff --git a/src/mongo/db/s/sharding_write_router_bm.cpp b/src/mongo/db/s/sharding_write_router_bm.cpp index 6029c016536..ddaaf5ce43c 100644 --- a/src/mongo/db/s/sharding_write_router_bm.cpp +++ b/src/mongo/db/s/sharding_write_router_bm.cpp @@ -48,6 +48,7 @@ #include "mongo/s/catalog_cache_loader_mock.h" #include "mongo/s/catalog_cache_mock.h" #include "mongo/s/chunk_manager.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/unittest/unittest.h" #include "mongo/util/assert_util.h" #include "mongo/util/processinfo.h" @@ -148,12 +149,13 @@ std::unique_ptr<CatalogCacheMock> createCatalogCacheMock(OperationContext* opCtx opCtx->getServiceContext(), std::make_unique<CollectionShardingStateFactoryShard>(opCtx->getServiceContext())); - const ChunkVersion placementVersion = chunkManager.getVersion(originatorShard); OperationShardingState::setShardRole( opCtx, kNss, - ShardVersion(placementVersion, - boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, + ShardVersionFactory::make( + chunkManager, + originatorShard, + boost::optional<CollectionIndexes>(boost::none)) /* shardVersion */, boost::none /* databaseVersion */); // Configuring the filtering metadata such that calls to getCollectionDescription return what we diff --git a/src/mongo/db/s/split_chunk.cpp b/src/mongo/db/s/split_chunk.cpp index 1b996db0af6..1005ab64cbf 100644 --- a/src/mongo/db/s/split_chunk.cpp +++ b/src/mongo/db/s/split_chunk.cpp @@ -47,6 +47,7 @@ #include "mongo/s/catalog/type_chunk.h" #include "mongo/s/client/shard_registry.h" #include "mongo/s/grid.h" +#include "mongo/s/shard_version_factory.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kSharding @@ -120,15 +121,15 @@ bool checkMetadataForSuccessfulSplitChunk(OperationContext* opCtx, metadataAfterSplit->isSharded()); const auto placementVersion = metadataAfterSplit->getShardVersion(); const auto epoch = placementVersion.epoch(); - uassert( - StaleConfigInfo(nss, - ShardVersion::IGNORED() /* receivedVersion */, - ShardVersion(placementVersion, - scopedCSR->getCollectionIndexes(opCtx)) /* wantedVersion */, - shardId), - str::stream() << "Collection " << nss.ns() << " changed since split start", - epoch == expectedEpoch && - (!expectedTimestamp || placementVersion.getTimestamp() == expectedTimestamp)); + uassert(StaleConfigInfo(nss, + ShardVersion::IGNORED() /* receivedVersion */, + ShardVersionFactory::make( + *metadataAfterSplit, + scopedCSR->getCollectionIndexes(opCtx)) /* wantedVersion */, + shardId), + str::stream() << "Collection " << nss.ns() << " changed since split start", + epoch == expectedEpoch && + (!expectedTimestamp || placementVersion.getTimestamp() == expectedTimestamp)); ChunkType nextChunk; for (auto it = splitPoints.begin(); it != splitPoints.end(); ++it) { diff --git a/src/mongo/db/shard_role_test.cpp b/src/mongo/db/shard_role_test.cpp index 1f5384e86ac..7e6565fef4f 100644 --- a/src/mongo/db/shard_role_test.cpp +++ b/src/mongo/db/shard_role_test.cpp @@ -38,6 +38,7 @@ #include "mongo/db/s/sharding_state.h" #include "mongo/db/service_context_d_test_fixture.h" #include "mongo/db/shard_role.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/unittest/death_test.h" #include "mongo/unittest/unittest.h" @@ -131,9 +132,9 @@ protected: const NamespaceString nssShardedCollection1 = NamespaceString::createNamespaceString_forTest(dbNameTestDb, "sharded"); - const ShardVersion shardVersionShardedCollection1{ + const ShardVersion shardVersionShardedCollection1 = ShardVersionFactory::make( ChunkVersion(CollectionGeneration{OID::gen(), Timestamp(5, 0)}, CollectionPlacement(10, 1)), - boost::optional<CollectionIndexes>(boost::none)}; + boost::optional<CollectionIndexes>(boost::none)); // Workaround to be able to write parametrized TEST_F void testRestoreFailsIfCollectionNoLongerExists( @@ -798,7 +799,8 @@ TEST_F(ShardRoleTest, RestoreForWriteFailsIfPlacementConcernNoLongerMet) { const auto newShardVersion = [&]() { auto newPlacementVersion = shardVersionShardedCollection1.placementVersion(); newPlacementVersion.incMajor(); - return ShardVersion(newPlacementVersion, boost::optional<CollectionIndexes>(boost::none)); + return ShardVersionFactory::make(newPlacementVersion, + boost::optional<CollectionIndexes>(boost::none)); }(); const auto uuid = getCollectionUUID(opCtx(), nss); installShardedCollectionMetadata( @@ -853,7 +855,8 @@ TEST_F(ShardRoleTest, RestoreWithShardVersionIgnored) { const auto newShardVersion = [&]() { auto newPlacementVersion = shardVersionShardedCollection1.placementVersion(); newPlacementVersion.incMajor(); - return ShardVersion(newPlacementVersion, boost::optional<CollectionIndexes>(boost::none)); + return ShardVersionFactory::make(newPlacementVersion, + boost::optional<CollectionIndexes>(boost::none)); }(); const auto uuid = getCollectionUUID(opCtx(), nss); @@ -1025,8 +1028,8 @@ TEST_F(ShardRoleTest, RestoreForReadSucceedsEvenIfPlacementHasChanged) { const auto newShardVersion = [&]() { auto newPlacementVersion = shardVersionShardedCollection1.placementVersion(); newPlacementVersion.incMajor(); - return ShardVersion(newPlacementVersion, - boost::optional<CollectionIndexes>(boost::none)); + return ShardVersionFactory::make(newPlacementVersion, + boost::optional<CollectionIndexes>(boost::none)); }(); const auto uuid = getCollectionUUID(opCtx(), nss); diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript index db5eccbab0a..a243129bbd6 100644 --- a/src/mongo/s/SConscript +++ b/src/mongo/s/SConscript @@ -267,6 +267,7 @@ env.Library( 'shard_key_pattern.cpp', 'shard_version.cpp', 'shard_version.idl', + 'shard_version_factory.cpp', 'sharding_feature_flags.idl', 'stale_exception.cpp', 'type_collection_common_types.idl', diff --git a/src/mongo/s/append_raw_responses_test.cpp b/src/mongo/s/append_raw_responses_test.cpp index 00d9c94d1df..7124c2ea4f8 100644 --- a/src/mongo/s/append_raw_responses_test.cpp +++ b/src/mongo/s/append_raw_responses_test.cpp @@ -34,6 +34,7 @@ #include "mongo/s/catalog/sharding_catalog_client_mock.h" #include "mongo/s/catalog/type_shard.h" #include "mongo/s/cluster_commands_helpers.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_router_test_fixture.h" #include "mongo/unittest/unittest.h" @@ -196,11 +197,12 @@ protected: [] { OID epoch{OID::gen()}; Timestamp timestamp{1, 0}; - return StaleConfigInfo(NamespaceString::createNamespaceString_forTest("Foo.Bar"), - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none, - ShardId{"dummy"}); + return StaleConfigInfo( + NamespaceString::createNamespaceString_forTest("Foo.Bar"), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none, + ShardId{"dummy"}); }(), "dummy"}; diff --git a/src/mongo/s/catalog_cache.cpp b/src/mongo/s/catalog_cache.cpp index 96ea5a1937d..e98c2ffbc55 100644 --- a/src/mongo/s/catalog_cache.cpp +++ b/src/mongo/s/catalog_cache.cpp @@ -45,6 +45,7 @@ #include "mongo/s/is_mongos.h" #include "mongo/s/mongod_and_mongos_server_parameters_gen.h" #include "mongo/s/shard_cannot_refresh_due_to_locks_held_exception.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_feature_flags_gen.h" #include "mongo/s/stale_exception.h" #include "mongo/util/concurrency/with_lock.h" @@ -168,16 +169,40 @@ std::shared_ptr<RoutingTableHistory> createUpdatedRoutingTableHistory( return std::make_shared<RoutingTableHistory>(std::move(newRoutingHistory)); } +StatusWith<CollectionRoutingInfo> retryUntilConsistentRoutingInfo( + OperationContext* opCtx, + const NamespaceString& nss, + ChunkManager&& cm, + boost::optional<GlobalIndexesCache>&& gii) { + const auto catalogCache = Grid::get(opCtx)->catalogCache(); + try { + // A non-empty GlobalIndexesCache implies that the collection is sharded since global + // indexes cannot be created on unsharded collections. + while (gii && (!cm.isSharded() || !cm.uuidMatches(gii->getCollectionIndexes().uuid()))) { + auto nextGii = catalogCache->getCollectionIndexInfoWithRefresh(opCtx, nss); + if (gii.is_initialized() && nextGii.is_initialized() && + gii->getCollectionIndexes() == nextGii->getCollectionIndexes()) { + cm = uassertStatusOK( + catalogCache->getCollectionPlacementInfoWithRefresh(opCtx, nss)); + } + gii = std::move(nextGii); + } + } catch (const DBException& ex) { + return ex.toStatus(); + } + return CollectionRoutingInfo{std::move(cm), std::move(gii)}; +} + } // namespace ShardVersion CollectionRoutingInfo::getCollectionVersion() const { - return ShardVersion(cm.getVersion(), - gii ? boost::make_optional(gii->getCollectionIndexes()) : boost::none); + return ShardVersionFactory::make( + cm, gii ? boost::make_optional(gii->getCollectionIndexes()) : boost::none); } ShardVersion CollectionRoutingInfo::getShardVersion(const ShardId& shardId) const { - return ShardVersion(cm.getVersion(shardId), - gii ? boost::make_optional(gii->getCollectionIndexes()) : boost::none); + return ShardVersionFactory::make( + cm, shardId, gii ? boost::make_optional(gii->getCollectionIndexes()) : boost::none); } AtomicWord<uint64_t> ComparableDatabaseVersion::_disambiguatingSequenceNumSource{1ULL}; @@ -426,7 +451,7 @@ StatusWith<CollectionRoutingInfo> CatalogCache::getCollectionRoutingInfoAt( try { auto cm = uassertStatusOK(getCollectionPlacementInfoAt(opCtx, nss, atClusterTime)); auto gii = getCollectionIndexInfoAt(opCtx, nss, atClusterTime); - return CollectionRoutingInfo{std::move(cm), std::move(gii)}; + return retryUntilConsistentRoutingInfo(opCtx, nss, std::move(cm), std::move(gii)); } catch (const DBException& ex) { return ex.toStatus(); } @@ -438,7 +463,7 @@ StatusWith<CollectionRoutingInfo> CatalogCache::getCollectionRoutingInfo(Operati try { auto cm = uassertStatusOK(getCollectionPlacementInfo(opCtx, nss, allowLocks)); auto gii = getCollectionIndexInfo(opCtx, nss, allowLocks); - return CollectionRoutingInfo{std::move(cm), std::move(gii)}; + return retryUntilConsistentRoutingInfo(opCtx, nss, std::move(cm), std::move(gii)); } catch (const DBException& ex) { return ex.toStatus(); } @@ -564,7 +589,7 @@ StatusWith<CollectionRoutingInfo> CatalogCache::getCollectionRoutingInfoWithRefr try { auto cm = uassertStatusOK(getCollectionPlacementInfoWithRefresh(opCtx, nss)); auto gii = getCollectionIndexInfoWithRefresh(opCtx, nss); - return CollectionRoutingInfo(std::move(cm), std::move(gii)); + return retryUntilConsistentRoutingInfo(opCtx, nss, std::move(cm), std::move(gii)); } catch (const DBException& ex) { return ex.toStatus(); } @@ -595,7 +620,12 @@ CollectionRoutingInfo CatalogCache::getShardedCollectionRoutingInfo(OperationCon const NamespaceString& nss) { auto cm = getShardedCollectionPlacementInfo(opCtx, nss); auto gii = getCollectionIndexInfo(opCtx, nss); - return CollectionRoutingInfo(std::move(cm), std::move(gii)); + auto cri = + uassertStatusOK(retryUntilConsistentRoutingInfo(opCtx, nss, std::move(cm), std::move(gii))); + uassert(ErrorCodes::NamespaceNotSharded, + str::stream() << "Expected collection " << nss << " to be sharded", + cri.cm.isSharded()); + return cri; } StatusWith<CollectionRoutingInfo> CatalogCache::getShardedCollectionRoutingInfoWithRefresh( @@ -603,7 +633,12 @@ StatusWith<CollectionRoutingInfo> CatalogCache::getShardedCollectionRoutingInfoW try { auto cm = uassertStatusOK(getShardedCollectionPlacementInfoWithRefresh(opCtx, nss)); auto gii = getCollectionIndexInfoWithRefresh(opCtx, nss); - return CollectionRoutingInfo(std::move(cm), std::move(gii)); + auto cri = uassertStatusOK( + retryUntilConsistentRoutingInfo(opCtx, nss, std::move(cm), std::move(gii))); + uassert(ErrorCodes::NamespaceNotSharded, + str::stream() << "Expected collection " << nss << " to be sharded", + cri.cm.isSharded()); + return cri; } catch (const DBException& ex) { return ex.toStatus(); } diff --git a/src/mongo/s/catalog_cache_test.cpp b/src/mongo/s/catalog_cache_test.cpp index 50111fd8784..0e957062607 100644 --- a/src/mongo/s/catalog_cache_test.cpp +++ b/src/mongo/s/catalog_cache_test.cpp @@ -32,6 +32,7 @@ #include "mongo/s/catalog/type_database_gen.h" #include "mongo/s/catalog_cache.h" #include "mongo/s/catalog_cache_loader_mock.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_router_test_fixture.h" #include "mongo/s/stale_exception.h" #include "mongo/s/type_collection_common_types_gen.h" @@ -288,8 +289,8 @@ TEST_F(CatalogCacheTest, OnStaleDatabaseVersionNoVersion) { TEST_F(CatalogCacheTest, OnStaleShardVersionWithSameVersion) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - const auto cachedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto cachedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); loadCollection(cachedCollVersion); @@ -301,8 +302,8 @@ TEST_F(CatalogCacheTest, OnStaleShardVersionWithSameVersion) { TEST_F(CatalogCacheTest, OnStaleShardVersionWithNoVersion) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - const auto cachedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto cachedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); loadCollection(cachedCollVersion); @@ -316,10 +317,10 @@ TEST_F(CatalogCacheTest, OnStaleShardVersionWithNoVersion) { TEST_F(CatalogCacheTest, OnStaleShardVersionWithGreaterPlacementVersion) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - const auto cachedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); - const auto wantedCollVersion = - ShardVersion(ChunkVersion(gen, {2, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto cachedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto wantedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {2, 0}), boost::optional<CollectionIndexes>(boost::none)); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); loadCollection(cachedCollVersion); @@ -333,8 +334,8 @@ TEST_F(CatalogCacheTest, OnStaleShardVersionWithGreaterPlacementVersion) { TEST_F(CatalogCacheTest, TimeseriesFieldsAreProperlyPropagatedOnCC) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const auto gen = CollectionGeneration(OID::gen(), Timestamp(42)); - const auto version = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto version = ShardVersionFactory::make(ChunkVersion(gen, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); @@ -391,8 +392,8 @@ TEST_F(CatalogCacheTest, TimeseriesFieldsAreProperlyPropagatedOnCC) { TEST_F(CatalogCacheTest, LookupCollectionWithInvalidOptions) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const auto gen = CollectionGeneration(OID::gen(), Timestamp(42)); - const auto version = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto version = ShardVersionFactory::make(ChunkVersion(gen, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); @@ -412,10 +413,10 @@ TEST_F(CatalogCacheTest, LookupCollectionWithInvalidOptions) { TEST_F(CatalogCacheTest, OnStaleShardVersionWithGreaterIndexVersion) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - const auto cachedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); - const auto wantedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(1, 0))); + const auto cachedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); + const auto wantedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(1, 0))); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); CollectionType coll = loadCollection(cachedCollVersion); @@ -439,10 +440,10 @@ TEST_F(CatalogCacheTest, OnStaleShardVersionWithGreaterIndexVersion) { TEST_F(CatalogCacheTest, OnStaleShardVersionIndexVersionBumpNotNone) { const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1)); const CollectionGeneration gen(OID::gen(), Timestamp(1, 1)); - const auto cachedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(1, 0))); - const auto wantedCollVersion = - ShardVersion(ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(2, 0))); + const auto cachedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(1, 0))); + const auto wantedCollVersion = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), CollectionIndexes(kUUID, Timestamp(2, 0))); loadDatabases({DatabaseType(kNss.db().toString(), kShards[0], dbVersion)}); CollectionType coll = loadCollection(cachedCollVersion); diff --git a/src/mongo/s/collection_routing_info_targeter.cpp b/src/mongo/s/collection_routing_info_targeter.cpp index d56281a9d4e..28cd753fc85 100644 --- a/src/mongo/s/collection_routing_info_targeter.cpp +++ b/src/mongo/s/collection_routing_info_targeter.cpp @@ -270,11 +270,12 @@ CollectionRoutingInfo CollectionRoutingInfoTargeter::_init(OperationContext* opC uassertStatusOK(Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfoWithRefresh( opCtx, bucketsNs)); } - auto [bucketsPlacementInfo, _] = + auto [bucketsPlacementInfo, bucketsIndexInfo] = uassertStatusOK(getCollectionRoutingInfoForTxnCmd(opCtx, bucketsNs)); if (bucketsPlacementInfo.isSharded()) { _nss = bucketsNs; cm = std::move(bucketsPlacementInfo); + gii = std::move(bucketsIndexInfo); _isRequestOnTimeseriesViewNamespace = true; } } else if (!cm.isSharded() && _isRequestOnTimeseriesViewNamespace) { @@ -286,9 +287,9 @@ CollectionRoutingInfo CollectionRoutingInfoTargeter::_init(OperationContext* opC uassertStatusOK( Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfoWithRefresh(opCtx, _nss)); } - auto cri = uassertStatusOK(getCollectionRoutingInfoForTxnCmd(opCtx, _nss)); - cm = cri.cm; - gii = cri.gii; + auto [newCm, newGii] = uassertStatusOK(getCollectionRoutingInfoForTxnCmd(opCtx, _nss)); + cm = std::move(newCm); + gii = std::move(newGii); _isRequestOnTimeseriesViewNamespace = false; } diff --git a/src/mongo/s/commands/cluster_analyze_shard_key_cmd.cpp b/src/mongo/s/commands/cluster_analyze_shard_key_cmd.cpp index 067dc496b0d..45935adb538 100644 --- a/src/mongo/s/commands/cluster_analyze_shard_key_cmd.cpp +++ b/src/mongo/s/commands/cluster_analyze_shard_key_cmd.cpp @@ -54,11 +54,7 @@ BSONObj makeVersionedCmdObj(const CollectionRoutingInfo& cri, const BSONObj& unversionedCmdObj, ShardId shardId) { if (cri.cm.isSharded()) { - return appendShardVersion( - unversionedCmdObj, - ShardVersion(cri.cm.getVersion(shardId), - cri.gii ? boost::make_optional(cri.gii->getCollectionIndexes()) - : boost::none)); + return appendShardVersion(unversionedCmdObj, cri.getShardVersion(shardId)); } auto versionedCmdObj = appendShardVersion(unversionedCmdObj, ShardVersion::UNSHARDED()); return appendDbVersionIfPresent(versionedCmdObj, cri.cm.dbVersion()); diff --git a/src/mongo/s/commands/cluster_split_cmd.cpp b/src/mongo/s/commands/cluster_split_cmd.cpp index 8d5ed3d68e9..0b8deb18b26 100644 --- a/src/mongo/s/commands/cluster_split_cmd.cpp +++ b/src/mongo/s/commands/cluster_split_cmd.cpp @@ -42,6 +42,7 @@ #include "mongo/s/grid.h" #include "mongo/s/shard_key_pattern_query_util.h" #include "mongo/s/shard_util.h" +#include "mongo/s/shard_version_factory.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kCommand @@ -56,18 +57,14 @@ BSONObj selectMedianKey(OperationContext* opCtx, const ShardId& shardId, const NamespaceString& nss, const ShardKeyPattern& shardKeyPattern, - const ChunkVersion& chunkVersion, + const CollectionRoutingInfo& cri, const ChunkRange& chunkRange) { - const auto gii = - Grid::get(opCtx)->catalogCache()->getCollectionIndexInfoWithRefresh(opCtx, nss); - ShardVersion shardVersion( - chunkVersion, gii ? boost::make_optional(gii->getCollectionIndexes()) : boost::none); BSONObjBuilder cmd; cmd.append("splitVector", nss.ns()); cmd.append("keyPattern", shardKeyPattern.toBSON()); chunkRange.append(&cmd); cmd.appendBool("force", true); - shardVersion.serialize(ShardVersion::kShardVersionField, &cmd); + cri.getShardVersion(shardId).serialize(ShardVersion::kShardVersionField, &cmd); auto shard = uassertStatusOK(Grid::get(opCtx)->shardRegistry()->getShard(opCtx, shardId)); @@ -136,9 +133,10 @@ public: BSONObjBuilder& result) override { const NamespaceString nss(parseNs({boost::none, dbname}, cmdObj)); - const auto cm = uassertStatusOK( - Grid::get(opCtx)->catalogCache()->getShardedCollectionPlacementInfoWithRefresh(opCtx, - nss)); + const auto cri = uassertStatusOK( + Grid::get(opCtx)->catalogCache()->getShardedCollectionRoutingInfoWithRefresh(opCtx, + nss)); + const auto& cm = cri.cm; const BSONField<BSONObj> findField("find", BSONObj()); const BSONField<BSONArray> boundsField("bounds", BSONArray()); @@ -257,7 +255,7 @@ public: chunk->getShardId(), nss, cm.getShardKeyPattern(), - cm.getVersion(chunk->getShardId()), + cri, ChunkRange(chunk->getMin(), chunk->getMax())); LOGV2(22758, diff --git a/src/mongo/s/commands/sharding_expressions.cpp b/src/mongo/s/commands/sharding_expressions.cpp index 407dd622eab..67e1794d7a1 100644 --- a/src/mongo/s/commands/sharding_expressions.cpp +++ b/src/mongo/s/commands/sharding_expressions.cpp @@ -48,6 +48,7 @@ #include "mongo/db/s/sharding_state.h" #include "mongo/s/grid.h" #include "mongo/s/is_mongos.h" +#include "mongo/s/shard_version_factory.h" namespace mongo { namespace { @@ -386,16 +387,13 @@ Value ExpressionInternalOwningShard::evaluate(const Document& root, Variables* v // Setting 'allowLocks' to true when evaluating on mongod, as otherwise an invariant is thrown. // We can safely set it to true as there is no risk of deadlock, because the code still throws // when a refresh would actually need to take place. - const auto chunkManager = - uassertStatusOK(catalogCache->getCollectionRoutingInfo(opCtx, ns, true /* allowLocks */)) - .cm; + const auto cri = + uassertStatusOK(catalogCache->getCollectionRoutingInfo(opCtx, ns, true /* allowLocks */)); // Invalidate catalog cache if the chunk manager version is stale. - if (chunkManager.getVersion().isOlderThan(shardVersion.placementVersion())) { - boost::optional<CollectionIndexes> collIndexes; - ShardVersion currentShardVersion(chunkManager.getVersion(), collIndexes); + if (cri.cm.getVersion().isOlderThan(shardVersion.placementVersion())) { uasserted(StaleConfigInfo(ns, - currentShardVersion, + cri.getCollectionVersion(), boost::none /* wanted */, ShardingState::get(opCtx)->shardId()), str::stream() @@ -405,7 +403,7 @@ Value ExpressionInternalOwningShard::evaluate(const Document& root, Variables* v // Retrieve the shard id for the given shard key value. std::set<ShardId> shardIds; - chunkManager.getShardIdsForRange(shardKeyVal, shardKeyVal, &shardIds); + cri.cm.getShardIdsForRange(shardKeyVal, shardKeyVal, &shardIds); uassert(6868601, "The value should belong to exactly one ShardId", shardIds.size() == 1); const auto shardId = *(shardIds.begin()); return Value(shardId.toString()); diff --git a/src/mongo/s/router.cpp b/src/mongo/s/router.cpp index 180c9766bee..93d751936c9 100644 --- a/src/mongo/s/router.cpp +++ b/src/mongo/s/router.cpp @@ -105,11 +105,7 @@ CollectionRouter::CollectionRouter(ServiceContext* service, NamespaceString nss) void CollectionRouter::appendCRUDRoutingTokenToCommand(const ShardId& shardId, const CollectionRoutingInfo& cri, BSONObjBuilder* builder) { - auto chunkVersion(cri.cm.getVersion(shardId)); - auto collectionIndexes(cri.gii ? boost::make_optional(cri.gii->getCollectionIndexes()) - : boost::none); - - if (chunkVersion == ChunkVersion::UNSHARDED()) { + if (cri.cm.getVersion(shardId) == ChunkVersion::UNSHARDED()) { // Need to add the database version as well const auto& dbVersion = cri.cm.dbVersion(); if (!dbVersion.isFixed()) { @@ -117,8 +113,7 @@ void CollectionRouter::appendCRUDRoutingTokenToCommand(const ShardId& shardId, dbVersion.serialize(&dbvBuilder); } } - ShardVersion(chunkVersion, collectionIndexes) - .serialize(ShardVersion::kShardVersionField, builder); + cri.getShardVersion(shardId).serialize(ShardVersion::kShardVersionField, builder); } CollectionRoutingInfo CollectionRouter::_getRoutingInfo(OperationContext* opCtx) const { diff --git a/src/mongo/s/shard_version.h b/src/mongo/s/shard_version.h index afe5a80390b..d5cba5261b8 100644 --- a/src/mongo/s/shard_version.h +++ b/src/mongo/s/shard_version.h @@ -34,7 +34,8 @@ namespace mongo { /** - * This class is used to represent the shard version of a collection. + * This class is used to represent the shard version of a collection. Objects of this class can be + * constructed through the ShardVersionFactory. * * It contains the chunk placement information through the ChunkVersion. This class is used for * network requests and the shard versioning protocol. @@ -48,11 +49,6 @@ public: */ static constexpr StringData kShardVersionField = "shardVersion"_sd; - ShardVersion(ChunkVersion chunkVersion, boost::optional<CollectionIndexes> indexVersion) - : _chunkVersion(chunkVersion), - _indexVersion(indexVersion ? boost::make_optional(indexVersion->indexVersion()) - : boost::none) {} - ShardVersion() : _chunkVersion(ChunkVersion()), _indexVersion(boost::none) {} static ShardVersion IGNORED() { @@ -92,9 +88,17 @@ public: std::string toString() const; private: + ShardVersion(const ChunkVersion& chunkVersion, + const boost::optional<CollectionIndexes>& collectionIndexes) + : _chunkVersion(chunkVersion), + _indexVersion(collectionIndexes ? boost::make_optional(collectionIndexes->indexVersion()) + : boost::none) {} + ShardVersion(ChunkVersion chunkVersion, boost::optional<Timestamp> indexVersionTimestamp) : _chunkVersion(chunkVersion), _indexVersion(indexVersionTimestamp) {} + friend class ShardVersionFactory; + ChunkVersion _chunkVersion; boost::optional<Timestamp> _indexVersion; }; diff --git a/src/mongo/s/shard_version_factory.cpp b/src/mongo/s/shard_version_factory.cpp new file mode 100644 index 00000000000..2d2ef3641e6 --- /dev/null +++ b/src/mongo/s/shard_version_factory.cpp @@ -0,0 +1,75 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/s/shard_version_factory.h" + +namespace mongo { + +ShardVersion ShardVersionFactory::make( + const ChunkManager& chunkManager, const boost::optional<CollectionIndexes>& collectionIndexes) { + tassert(7288900, + str::stream() << "Cannot create ShardVersion when placement version has uuid " + << chunkManager.getUUID() << " and index version has uuid " + << collectionIndexes->uuid(), + !collectionIndexes || chunkManager.uuidMatches(collectionIndexes->uuid())); + return ShardVersion(chunkManager.getVersion(), collectionIndexes); +} + +ShardVersion ShardVersionFactory::make( + const ChunkManager& chunkManager, + const ShardId& shardId, + const boost::optional<CollectionIndexes>& collectionIndexes) { + + tassert(7288901, + str::stream() << "Cannot create ShardVersion when placement version has uuid " + << chunkManager.getUUID() << " and index version has uuid " + << collectionIndexes->uuid(), + !collectionIndexes || chunkManager.uuidMatches(collectionIndexes->uuid())); + return ShardVersion(chunkManager.getVersion(shardId), collectionIndexes); +} + +ShardVersion ShardVersionFactory::make( + const CollectionMetadata& cm, const boost::optional<CollectionIndexes>& collectionIndexes) { + tassert(7288902, + str::stream() << "Cannot create ShardVersion when placement version has uuid " + << cm.getUUID() << " and index version has uuid " + << collectionIndexes->uuid(), + !collectionIndexes || !cm.isSharded() || cm.uuidMatches(collectionIndexes->uuid())); + return ShardVersion(cm.getShardVersion(), collectionIndexes); +} + + +// The other three constructors should be used instead of this one whenever possible. This +// builder should only be used for the rare cases in which we know that the chunk version and +// collection indexes come from the same collection. +ShardVersion ShardVersionFactory::make( + const ChunkVersion& chunkVersion, const boost::optional<CollectionIndexes>& collectionIndexes) { + return ShardVersion(chunkVersion, collectionIndexes); +} +} // namespace mongo diff --git a/src/mongo/s/shard_version_factory.h b/src/mongo/s/shard_version_factory.h new file mode 100644 index 00000000000..1b3884c1810 --- /dev/null +++ b/src/mongo/s/shard_version_factory.h @@ -0,0 +1,58 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ +#pragma once + +#include "mongo/db/s/collection_metadata.h" +#include "mongo/s/shard_version.h" + +namespace mongo { + +/** + * This class is used to build shard version objects. + */ +class ShardVersionFactory { +public: + static ShardVersion make(const ChunkManager& chunkManager, + const boost::optional<CollectionIndexes>& collectionIndexes); + + static ShardVersion make(const ChunkManager& chunkManager, + const ShardId& shardId, + const boost::optional<CollectionIndexes>& collectionIndexes); + + static ShardVersion make(const CollectionMetadata& cm, + const boost::optional<CollectionIndexes>& collectionIndexes); + + // The other three builders should be used instead of this one whenever possible. This + // builder should only be used for the rare cases in which we know that the chunk version and + // collection indexes come from the same collection. + static ShardVersion make(const ChunkVersion& chunkVersion, + const boost::optional<CollectionIndexes>& collectionIndexes); +}; + +} // namespace mongo diff --git a/src/mongo/s/shard_version_test.cpp b/src/mongo/s/shard_version_test.cpp index 8fe4654dada..9c40bddaf5b 100644 --- a/src/mongo/s/shard_version_test.cpp +++ b/src/mongo/s/shard_version_test.cpp @@ -27,6 +27,7 @@ * it in the license file. */ #include "mongo/s/shard_version.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/unittest/unittest.h" namespace mongo { @@ -36,7 +37,7 @@ TEST(ShardVersionTest, ConstructCorrectly) { const CollectionGeneration gen(OID::gen(), Timestamp(1, 2)); const ChunkVersion chunkVersion(gen, {3, 4}); const CollectionIndexes collectionIndexes(UUID::gen(), Timestamp(5, 6)); - const ShardVersion shardVersion(chunkVersion, collectionIndexes); + const ShardVersion shardVersion = ShardVersionFactory::make(chunkVersion, collectionIndexes); ASSERT_EQ(shardVersion.placementVersion().getTimestamp(), Timestamp(1, 2)); ASSERT_EQ(shardVersion.placementVersion().majorVersion(), 3); ASSERT_EQ(shardVersion.placementVersion().minorVersion(), 4); @@ -47,7 +48,7 @@ TEST(ShardVersionTest, ToAndFromBSON) { const CollectionGeneration gen(OID::gen(), Timestamp(1, 2)); const ChunkVersion chunkVersion(gen, {3, 4}); const CollectionIndexes collectionIndexes(UUID::gen(), Timestamp(5, 6)); - const ShardVersion shardVersion(chunkVersion, collectionIndexes); + const ShardVersion shardVersion = ShardVersionFactory::make(chunkVersion, collectionIndexes); BSONObjBuilder builder; shardVersion.serialize(ShardVersion::kShardVersionField, &builder); diff --git a/src/mongo/s/stale_shard_version_helpers_test.cpp b/src/mongo/s/stale_shard_version_helpers_test.cpp index 8bd43224eda..980977d2e74 100644 --- a/src/mongo/s/stale_shard_version_helpers_test.cpp +++ b/src/mongo/s/stale_shard_version_helpers_test.cpp @@ -28,6 +28,7 @@ */ #include "mongo/logv2/log.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_router_test_fixture.h" #include "mongo/s/stale_shard_version_helpers.h" #include "mongo/unittest/unittest.h" @@ -96,12 +97,13 @@ TEST_F(AsyncShardVersionRetry, LimitedStaleErrorsShouldReturnCorrectValue) { const CollectionGeneration gen1(OID::gen(), Timestamp(1, 0)); const CollectionGeneration gen2(OID::gen(), Timestamp(1, 0)); uassert( - StaleConfigInfo(nss(), - ShardVersion(ChunkVersion(gen1, {5, 23}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion(gen2, {6, 99}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId("sB")), + StaleConfigInfo( + nss(), + ShardVersionFactory::make(ChunkVersion(gen1, {5, 23}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen2, {6, 99}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId("sB")), "testX", false); } diff --git a/src/mongo/s/write_ops/batch_write_exec_test.cpp b/src/mongo/s/write_ops/batch_write_exec_test.cpp index bc1ffdb1ff0..e32d7579c96 100644 --- a/src/mongo/s/write_ops/batch_write_exec_test.cpp +++ b/src/mongo/s/write_ops/batch_write_exec_test.cpp @@ -37,6 +37,7 @@ #include "mongo/s/client/shard_registry.h" #include "mongo/s/mock_ns_targeter.h" #include "mongo/s/session_catalog_router.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_router_test_fixture.h" #include "mongo/s/stale_exception.h" #include "mongo/s/transaction_router.h" @@ -91,12 +92,13 @@ BSONObj expectInsertsReturnStaleVersionErrorsBase(const NamespaceString& nss, for (itInserted = inserted.begin(); itInserted != inserted.end(); ++itInserted) { staleResponse.addToErrDetails(write_ops::WriteError( i, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {2, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName1)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {2, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName1)), "Stale error"))); ++i; } @@ -335,10 +337,11 @@ public: const CollectionGeneration gen{OID::gen(), Timestamp(1, 1)}; MockNSTargeter singleShardNSTargeter{ nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion(gen, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion(gen, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << MAXKEY))}}; }; @@ -411,26 +414,28 @@ TEST_F(BatchWriteExecTest, SingleUpdateTargetsShardWithLet) { const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); @@ -507,26 +512,28 @@ TEST_F(BatchWriteExecTest, SingleDeleteTargetsShardWithLet) { const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, Timestamp(1, 1)}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, Timestamp(1, 1)}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, Timestamp(1, 1)}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, Timestamp(1, 1)}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, Timestamp(1, 1)}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, Timestamp(1, 1)}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); @@ -709,30 +716,33 @@ TEST_F(BatchWriteExecTest, StaleShardVersionReturnedFromBatchWithSingleMultiWrit const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); @@ -763,12 +773,13 @@ TEST_F(BatchWriteExecTest, StaleShardVersionReturnedFromBatchWithSingleMultiWrit response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 0, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -822,30 +833,33 @@ TEST_F(BatchWriteExecTest, const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << MINKEY), BSON("sk" << 10)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << 10), BSON("sk" << MAXKEY))}); @@ -876,21 +890,23 @@ TEST_F(BatchWriteExecTest, response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 0, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); response.addToErrDetails(write_ops::WriteError( 1, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -943,30 +959,33 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1Firs) { const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << MINKEY), BSON("sk" << 10)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << 10), BSON("sk" << MAXKEY))}); @@ -987,12 +1006,13 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1Firs) { response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 1, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -1005,12 +1025,13 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1Firs) { response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 0, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -1074,30 +1095,33 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1FirstOK const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << MINKEY), BSON("sk" << 10)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << 10), BSON("sk" << MAXKEY))}); @@ -1118,12 +1142,13 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1FirstOK response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 1, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -1136,12 +1161,13 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromMultiWriteWithShard1FirstOK response.setNModified(0); response.addToErrDetails(write_ops::WriteError( 1, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); return response.toBSON(); }); @@ -1202,20 +1228,22 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromWriteWithShard1SSVShard2OK) invariant(chunkRange == nullptr); if (targetAll) { return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } else { - return std::vector{ - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } } @@ -1224,16 +1252,18 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromWriteWithShard1SSVShard2OK) MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << MINKEY), BSON("sk" << 10)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("sk" << 10), BSON("sk" << MAXKEY))}); @@ -1255,12 +1285,13 @@ TEST_F(BatchWriteExecTest, RetryableErrorReturnedFromWriteWithShard1SSVShard2OK) response.setN(0); response.addToErrDetails(write_ops::WriteError( 0, - Status(StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId(kShardName2)), + Status(StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId(kShardName2)), "Stale error"))); // This simulates a migration of the last chunk on shard 1 to shard 2, which means that @@ -1979,30 +2010,33 @@ TEST_F(BatchWriteExecTargeterErrorTest, TargetedFailedAndErrorResponse) { OperationContext* opCtx, const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRanges = nullptr) const override { - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); @@ -2128,30 +2162,33 @@ TEST_F(BatchWriteExecTransactionTargeterErrorTest, TargetedFailedAndErrorRespons const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); @@ -2285,30 +2322,33 @@ TEST_F(BatchWriteExecTransactionMultiShardTest, TargetedSucceededAndErrorRespons const BatchItemRef& itemRef, std::set<ChunkRange>* chunkRange = nullptr) const override { invariant(chunkRange == nullptr); - return std::vector{ - ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), - ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none)}; + return std::vector{ShardEndpoint(kShardName1, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), + ShardEndpoint(kShardName2, + ShardVersionFactory::make( + ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none)}; } }; MultiShardTargeter multiShardNSTargeter( nss, - {MockRange(ShardEndpoint(kShardName1, - ShardVersion(ChunkVersion({epoch, timestamp}, {100, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + {MockRange(ShardEndpoint( + kShardName1, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {100, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << MINKEY), BSON("x" << 0)), - MockRange(ShardEndpoint(kShardName2, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none), + MockRange(ShardEndpoint( + kShardName2, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none), BSON("x" << 0), BSON("x" << MAXKEY))}); diff --git a/src/mongo/s/write_ops/batch_write_op_test.cpp b/src/mongo/s/write_ops/batch_write_op_test.cpp index 46bd5a26c79..c4990a6a8cc 100644 --- a/src/mongo/s/write_ops/batch_write_op_test.cpp +++ b/src/mongo/s/write_ops/batch_write_op_test.cpp @@ -32,6 +32,7 @@ #include "mongo/s/concurrency/locker_mongos_client_observer.h" #include "mongo/s/mock_ns_targeter.h" #include "mongo/s/session_catalog_router.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/sharding_router_test_fixture.h" #include "mongo/s/transaction_router.h" #include "mongo/s/write_ops/batch_write_op.h" @@ -288,12 +289,13 @@ TEST_F(BatchWriteOpTest, SingleStaleError) { Timestamp timestamp{1, 0}; response.addToErrDetails(write_ops::WriteError( 0, - Status{StaleConfigInfo(nss, - ShardVersion(ChunkVersion({epoch, timestamp}, {101, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, timestamp}, {105, 200}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId("shard")), + Status{StaleConfigInfo( + nss, + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {101, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {105, 200}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId("shard")), "mock stale error"})); // First stale response comes back, we should retry diff --git a/src/mongo/s/write_ops/batched_command_request_test.cpp b/src/mongo/s/write_ops/batched_command_request_test.cpp index 6408a9c6084..b8fa16ad9bc 100644 --- a/src/mongo/s/write_ops/batched_command_request_test.cpp +++ b/src/mongo/s/write_ops/batched_command_request_test.cpp @@ -29,6 +29,7 @@ #include "mongo/bson/json.h" #include "mongo/db/ops/write_ops_parsers_test_helpers.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/write_ops/batched_command_request.h" #include "mongo/unittest/unittest.h" @@ -72,8 +73,8 @@ TEST(BatchedCommandRequest, InsertWithShardVersion) { ASSERT_EQ("TestDB.test", insertRequest.getInsertRequest().getNamespace().ns()); ASSERT(insertRequest.hasShardVersion()); - ASSERT_EQ(ShardVersion(ChunkVersion({epoch, timestamp}, {1, 2}), - boost::optional<CollectionIndexes>(boost::none)) + ASSERT_EQ(ShardVersionFactory::make(ChunkVersion({epoch, timestamp}, {1, 2}), + boost::optional<CollectionIndexes>(boost::none)) .toString(), insertRequest.getShardVersion().toString()); } diff --git a/src/mongo/s/write_ops/batched_command_response_test.cpp b/src/mongo/s/write_ops/batched_command_response_test.cpp index e4eb5f3b447..0b6eee16eb5 100644 --- a/src/mongo/s/write_ops/batched_command_response_test.cpp +++ b/src/mongo/s/write_ops/batched_command_response_test.cpp @@ -29,6 +29,7 @@ #include "mongo/db/jsobj.h" #include "mongo/db/ops/write_ops.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/stale_exception.h" #include "mongo/s/write_ops/batched_command_response.h" #include "mongo/unittest/unittest.h" @@ -70,12 +71,14 @@ TEST(BatchedCommandResponseTest, Basic) { TEST(BatchedCommandResponseTest, StaleConfigInfo) { OID epoch = OID::gen(); - StaleConfigInfo staleInfo(NamespaceString::createNamespaceString_forTest("TestDB.TestColl"), - ShardVersion(ChunkVersion({epoch, Timestamp(100, 0)}, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion({epoch, Timestamp(100, 0)}, {2, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId("TestShard")); + StaleConfigInfo staleInfo( + NamespaceString::createNamespaceString_forTest("TestDB.TestColl"), + ShardVersionFactory::make(ChunkVersion({epoch, Timestamp(100, 0)}, {1, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion({epoch, Timestamp(100, 0)}, {2, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId("TestShard")); + BSONObjBuilder builder(BSON("index" << 0 << "code" << ErrorCodes::StaleConfig << "errmsg" << "StaleConfig error")); staleInfo.serialize(&builder); @@ -159,8 +162,8 @@ TEST(BatchedCommandResponseTest, TooManyBigErrors) { TEST(BatchedCommandResponseTest, CompatibilityFromWriteErrorToBatchCommandResponse) { CollectionGeneration gen(OID::gen(), Timestamp(2, 0)); - ShardVersion versionReceived(ChunkVersion(gen, {1, 0}), - boost::optional<CollectionIndexes>(boost::none)); + const auto versionReceived = ShardVersionFactory::make( + ChunkVersion(gen, {1, 0}), boost::optional<CollectionIndexes>(boost::none)); write_ops::UpdateCommandReply reply; reply.getWriteCommandReplyBase().setN(1); diff --git a/src/mongo/s/write_ops/write_op_test.cpp b/src/mongo/s/write_ops/write_op_test.cpp index ef067bdb55e..c89cc94c70b 100644 --- a/src/mongo/s/write_ops/write_op_test.cpp +++ b/src/mongo/s/write_ops/write_op_test.cpp @@ -33,6 +33,7 @@ #include "mongo/s/concurrency/locker_mongos_client_observer.h" #include "mongo/s/mock_ns_targeter.h" #include "mongo/s/session_catalog_router.h" +#include "mongo/s/shard_version_factory.h" #include "mongo/s/transaction_router.h" #include "mongo/s/write_ops/write_op.h" #include "mongo/unittest/unittest.h" @@ -62,12 +63,13 @@ void sortByEndpoint(std::vector<std::unique_ptr<TargetedWrite>>* writes) { class WriteOpTest : public ServiceContextTest { protected: static Status getMockRetriableError(CollectionGeneration& gen) { - return {StaleConfigInfo(kNss, - ShardVersion(ChunkVersion(gen, {10, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardVersion(ChunkVersion(gen, {11, 0}), - boost::optional<CollectionIndexes>(boost::none)), - ShardId("shardA")), + return {StaleConfigInfo( + kNss, + ShardVersionFactory::make(ChunkVersion(gen, {10, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {11, 0}), + boost::optional<CollectionIndexes>(boost::none)), + ShardId("shardA")), "simulate ssv error for test"}; } @@ -80,14 +82,16 @@ protected: WriteOp setupTwoShardTest(CollectionGeneration& gen, std::vector<std::unique_ptr<TargetedWrite>>& targeted, bool isTransactional) const { - ShardEndpoint endpointA(ShardId("shardA"), - ShardVersion(ChunkVersion(gen, {10, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none); - ShardEndpoint endpointB(ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), - boost::optional<CollectionIndexes>(boost::none)), - boost::none); + ShardEndpoint endpointA( + ShardId("shardA"), + ShardVersionFactory::make(ChunkVersion(gen, {10, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none); + ShardEndpoint endpointB( + ShardId("shardB"), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), + boost::none); BatchedCommandRequest request([&] { write_ops::DeleteCommandRequest deleteOp(kNss); @@ -188,15 +192,18 @@ TEST_F(WriteOpTest, TargetMultiOneShard) { CollectionGeneration gen(OID(), Timestamp(1, 1)); ShardEndpoint endpointA( ShardId("shardA"), - ShardVersion(ChunkVersion(gen, {10, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {10, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointB( ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointC( ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); BatchedCommandRequest request([&] { @@ -230,15 +237,18 @@ TEST_F(WriteOpTest, TargetMultiAllShards) { CollectionGeneration gen(OID(), Timestamp(1, 1)); ShardEndpoint endpointA( ShardId("shardA"), - ShardVersion(ChunkVersion(gen, {10, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {10, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointB( ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointC( ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); BatchedCommandRequest request([&] { @@ -444,15 +454,18 @@ TEST_F(WriteOpTransactionTest, TargetMultiDoesNotTargetAllShards) { CollectionGeneration gen(OID(), Timestamp(1, 1)); ShardEndpoint endpointA( ShardId("shardA"), - ShardVersion(ChunkVersion(gen, {10, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {10, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointB( ShardId("shardB"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); ShardEndpoint endpointC( ShardId("shardC"), - ShardVersion(ChunkVersion(gen, {20, 0}), boost::optional<CollectionIndexes>(boost::none)), + ShardVersionFactory::make(ChunkVersion(gen, {20, 0}), + boost::optional<CollectionIndexes>(boost::none)), boost::none); BatchedCommandRequest request([&] { |