From 851dad7902d6bb8c3ed25f99f565a2e2c8c8bc47 Mon Sep 17 00:00:00 2001 From: Kaloian Manassiev Date: Tue, 26 Feb 2019 08:27:04 -0500 Subject: SERVER-39495 Only return versioned filtering metadata for cases that actually need to do filtering --- src/mongo/db/auth/auth_op_observer.cpp | 2 +- src/mongo/db/commands/count_cmd.cpp | 6 +- src/mongo/db/commands/mr.cpp | 2 +- src/mongo/db/exec/update_stage.cpp | 4 +- src/mongo/db/op_observer_impl.cpp | 2 +- src/mongo/db/pipeline/pipeline_d.cpp | 2 +- src/mongo/db/query/get_executor.cpp | 7 +- src/mongo/db/query/stage_builder.cpp | 10 +- src/mongo/db/s/collection_metadata.cpp | 110 ++++++++++----------- src/mongo/db/s/collection_metadata.h | 110 +++++++++++---------- .../db/s/collection_metadata_filtering_test.cpp | 6 +- src/mongo/db/s/collection_sharding_state.cpp | 27 ++--- src/mongo/db/s/collection_sharding_state.h | 26 +++-- src/mongo/db/s/get_shard_version_command.cpp | 2 +- src/mongo/db/s/op_observer_sharding_impl.cpp | 31 +++--- src/mongo/db/s/shard_server_op_observer.cpp | 4 +- 16 files changed, 185 insertions(+), 166 deletions(-) (limited to 'src/mongo/db') diff --git a/src/mongo/db/auth/auth_op_observer.cpp b/src/mongo/db/auth/auth_op_observer.cpp index 5a711fd7d20..df979414ba6 100644 --- a/src/mongo/db/auth/auth_op_observer.cpp +++ b/src/mongo/db/auth/auth_op_observer.cpp @@ -71,7 +71,7 @@ void AuthOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArg BSONObj AuthOpObserver::getDocumentKey(OperationContext* opCtx, NamespaceString const& nss, BSONObj const& doc) { - auto metadata = CollectionShardingState::get(opCtx, nss)->getMetadataForOperation(opCtx); + const auto metadata = CollectionShardingState::get(opCtx, nss)->getCurrentMetadata(); return metadata->extractDocumentKey(doc).getOwned(); } diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index 59c3c660dd9..312f8b44b13 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -159,8 +159,7 @@ public: // Prevent chunks from being cleaned up during yields - this allows us to only check the // version on initial entry into count. - auto rangePreserver = - CollectionShardingState::get(opCtx, nss)->getMetadataForOperation(opCtx); + auto rangePreserver = CollectionShardingState::get(opCtx, nss)->getCurrentMetadata(); auto statusWithPlanExecutor = getExecutorCount(opCtx, collection, request.getValue(), true /*explain*/); @@ -218,8 +217,7 @@ public: // Prevent chunks from being cleaned up during yields - this allows us to only check the // version on initial entry into count. - auto rangePreserver = - CollectionShardingState::get(opCtx, nss)->getMetadataForOperation(opCtx); + auto rangePreserver = CollectionShardingState::get(opCtx, nss)->getCurrentMetadata(); auto statusWithPlanExecutor = getExecutorCount(opCtx, collection, request.getValue(), false /*explain*/); diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 69f3e061ac2..dd933d76470 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -1421,7 +1421,7 @@ public: const auto metadata = [&] { AutoGetCollectionForReadCommand autoColl(opCtx, config.nss); - return CollectionShardingState::get(opCtx, config.nss)->getMetadataForOperation(opCtx); + return CollectionShardingState::get(opCtx, config.nss)->getCurrentMetadata(); }(); bool shouldHaveData = false; diff --git a/src/mongo/db/exec/update_stage.cpp b/src/mongo/db/exec/update_stage.cpp index f0b936005bc..f3f0970ed81 100644 --- a/src/mongo/db/exec/update_stage.cpp +++ b/src/mongo/db/exec/update_stage.cpp @@ -140,7 +140,7 @@ bool shouldRestartUpdateIfNoLongerMatches(const UpdateStageParams& params) { const std::vector>* getImmutableFields(OperationContext* opCtx, const NamespaceString& ns) { - auto metadata = CollectionShardingState::get(opCtx, ns)->getMetadataForOperation(opCtx); + auto metadata = CollectionShardingState::get(opCtx, ns)->getCurrentMetadata(); if (metadata->isSharded()) { const std::vector>& fields = metadata->getKeyPatternFields(); // Return shard-keys as immutable for the update system. @@ -304,7 +304,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco args.stmtId = request->getStmtId(); args.update = logObj; auto* const css = CollectionShardingState::get(getOpCtx(), collection()->ns()); - auto metadata = css->getMetadataForOperation(getOpCtx()); + auto metadata = css->getCurrentMetadata(); args.criteria = metadata->extractDocumentKey(newObj); uassert(16980, "Multi-update operations require all documents to have an '_id' field", diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index ec83552c415..502b8968dda 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -338,7 +338,7 @@ OpTimeBundle replLogApplyOps(OperationContext* opCtx, BSONObj OpObserverImpl::getDocumentKey(OperationContext* opCtx, NamespaceString const& nss, BSONObj const& doc) { - auto metadata = CollectionShardingState::get(opCtx, nss)->getMetadataForOperation(opCtx); + auto metadata = CollectionShardingState::get(opCtx, nss)->getCurrentMetadata(); return metadata->extractDocumentKey(doc).getOwned(); } diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp index 72d556508ea..773321d696c 100644 --- a/src/mongo/db/pipeline/pipeline_d.cpp +++ b/src/mongo/db/pipeline/pipeline_d.cpp @@ -128,7 +128,7 @@ StatusWith> createRandomCursorEx // collection, otherwise treat it as unsharded boost::optional shardMetadata = (OperationShardingState::isOperationVersioned(opCtx) - ? CollectionShardingState::get(opCtx, coll->ns())->getMetadataForOperation(opCtx) + ? CollectionShardingState::get(opCtx, coll->ns())->getOrphansFilter(opCtx) : boost::optional{}); // Because 'numRecords' includes orphan documents, our initial decision to optimize the $sample diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 12f62aea456..4da59c72cb8 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -243,8 +243,8 @@ void fillOutPlannerParams(OperationContext* opCtx, // If the caller wants a shard filter, make sure we're actually sharded. if (plannerParams->options & QueryPlannerParams::INCLUDE_SHARD_FILTER) { - auto collMetadata = CollectionShardingState::get(opCtx, canonicalQuery->nss()) - ->getMetadataForOperation(opCtx); + auto collMetadata = + CollectionShardingState::get(opCtx, canonicalQuery->nss())->getCurrentMetadata(); if (collMetadata->isSharded()) { plannerParams->shardKey = collMetadata->getKeyPattern(); } else { @@ -366,8 +366,7 @@ StatusWith prepareExecution(OperationContext* opCtx, if (plannerParams.options & QueryPlannerParams::INCLUDE_SHARD_FILTER) { root = make_unique( opCtx, - CollectionShardingState::get(opCtx, canonicalQuery->nss()) - ->getMetadataForOperation(opCtx), + CollectionShardingState::get(opCtx, canonicalQuery->nss())->getOrphansFilter(opCtx), ws, root.release()); } diff --git a/src/mongo/db/query/stage_builder.cpp b/src/mongo/db/query/stage_builder.cpp index c728955de5d..4612e58abfc 100644 --- a/src/mongo/db/query/stage_builder.cpp +++ b/src/mongo/db/query/stage_builder.cpp @@ -303,11 +303,11 @@ PlanStage* buildStages(OperationContext* opCtx, if (nullptr == childStage) { return nullptr; } - return new ShardFilterStage(opCtx, - CollectionShardingState::get(opCtx, collection->ns()) - ->getMetadataForOperation(opCtx), - ws, - childStage); + return new ShardFilterStage( + opCtx, + CollectionShardingState::get(opCtx, collection->ns())->getOrphansFilter(opCtx), + ws, + childStage); } case STAGE_DISTINCT_SCAN: { const DistinctNode* dn = static_cast(root); diff --git a/src/mongo/db/s/collection_metadata.cpp b/src/mongo/db/s/collection_metadata.cpp index 2949da0c9fa..a60729473c5 100644 --- a/src/mongo/db/s/collection_metadata.cpp +++ b/src/mongo/db/s/collection_metadata.cpp @@ -46,6 +46,46 @@ namespace mongo { CollectionMetadata::CollectionMetadata(std::shared_ptr cm, const ShardId& thisShardId) : _cm(std::move(cm)), _thisShardId(thisShardId) {} +BSONObj CollectionMetadata::extractDocumentKey(const BSONObj& doc) const { + BSONObj key; + + if (isSharded()) { + auto const& pattern = _cm->getShardKeyPattern(); + key = dotted_path_support::extractElementsBasedOnTemplate(doc, pattern.toBSON()); + if (pattern.hasId()) { + return key; + } + // else, try to append an _id field from the document. + } + + if (auto id = doc["_id"]) { + return key.isEmpty() ? id.wrap() : BSONObjBuilder(std::move(key)).append(id).obj(); + } + + // For legacy documents that lack an _id, use the document itself as its key. + return doc; +} + +void CollectionMetadata::toBSONBasic(BSONObjBuilder& bb) const { + if (isSharded()) { + _cm->getVersion().appendLegacyWithField(&bb, "collVersion"); + getShardVersion().appendLegacyWithField(&bb, "shardVersion"); + bb.append("keyPattern", _cm->getShardKeyPattern().toBSON()); + } else { + ChunkVersion::UNSHARDED().appendLegacyWithField(&bb, "collVersion"); + ChunkVersion::UNSHARDED().appendLegacyWithField(&bb, "shardVersion"); + } +} + +std::string CollectionMetadata::toStringBasic() const { + if (isSharded()) { + return str::stream() << "collection version: " << _cm->getVersion().toString() + << ", shard version: " << getShardVersion().toString(); + } else { + return "collection version: "; + } +} + RangeMap CollectionMetadata::getChunks() const { invariant(isSharded()); @@ -98,62 +138,8 @@ Status CollectionMetadata::checkChunkIsValid(const ChunkType& chunk) const { return Status::OK(); } -BSONObj CollectionMetadata::extractDocumentKey(BSONObj const& doc) const { - BSONObj key; - - if (isSharded()) { - auto const& pattern = getChunkManager()->getShardKeyPattern(); - key = dotted_path_support::extractElementsBasedOnTemplate(doc, pattern.toBSON()); - if (pattern.hasId()) { - return key; - } - // else, try to append an _id field from the document. - } - - if (auto id = doc["_id"]) { - return key.isEmpty() ? id.wrap() : BSONObjBuilder(std::move(key)).append(id).obj(); - } - - // For legacy documents that lack an _id, use the document itself as its key. - return doc; -} - -void CollectionMetadata::toBSONBasic(BSONObjBuilder& bb) const { - if (isSharded()) { - _cm->getVersion().appendLegacyWithField(&bb, "collVersion"); - getShardVersion().appendLegacyWithField(&bb, "shardVersion"); - bb.append("keyPattern", _cm->getShardKeyPattern().toBSON()); - } else { - ChunkVersion::UNSHARDED().appendLegacyWithField(&bb, "collVersion"); - ChunkVersion::UNSHARDED().appendLegacyWithField(&bb, "shardVersion"); - } -} - -void CollectionMetadata::toBSONChunks(BSONArrayBuilder& bb) const { - if (!isSharded()) - return; - - for (const auto& chunk : _cm->chunks()) { - if (chunk.getShardId() == _thisShardId) { - BSONArrayBuilder chunkBB(bb.subarrayStart()); - chunkBB.append(chunk.getMin()); - chunkBB.append(chunk.getMax()); - chunkBB.done(); - } - } -} - -std::string CollectionMetadata::toStringBasic() const { - if (isSharded()) { - return str::stream() << "collection version: " << _cm->getVersion().toString() - << ", shard version: " << getShardVersion().toString(); - } else { - return "collection version: "; - } -} - boost::optional CollectionMetadata::getNextOrphanRange( - RangeMap const& receivingChunks, BSONObj const& origLookupKey) const { + const RangeMap& receivingChunks, const BSONObj& origLookupKey) const { invariant(isSharded()); const BSONObj maxKey = getMaxKey(); @@ -221,4 +207,18 @@ boost::optional CollectionMetadata::getNextOrphanRange( return boost::none; } +void CollectionMetadata::toBSONChunks(BSONArrayBuilder* builder) const { + if (!isSharded()) + return; + + for (const auto& chunk : _cm->chunks()) { + if (chunk.getShardId() == _thisShardId) { + BSONArrayBuilder chunkBB(builder->subarrayStart()); + chunkBB.append(chunk.getMin()); + chunkBB.append(chunk.getMax()); + chunkBB.done(); + } + } +} + } // namespace mongo diff --git a/src/mongo/db/s/collection_metadata.h b/src/mongo/db/s/collection_metadata.h index 61af4fd8bc5..824787506f4 100644 --- a/src/mongo/db/s/collection_metadata.h +++ b/src/mongo/db/s/collection_metadata.h @@ -63,8 +63,7 @@ public: CollectionMetadata(std::shared_ptr cm, const ShardId& thisShardId); /** - * Returns whether this metadata object represents a sharded collection which requires - * filtering. + * Returns whether this metadata object represents a sharded or unsharded collection. */ bool isSharded() const { return bool(_cm); @@ -84,6 +83,47 @@ public: return (isSharded() ? _cm->getVersion() : ChunkVersion::UNSHARDED()); } + /** + * Obtains the shard id with which this collection metadata is configured. + */ + const ShardId& shardId() const { + invariant(isSharded()); + return _thisShardId; + } + + /** + * Returns true if 'key' contains exactly the same fields as the shard key pattern. + */ + bool isValidKey(const BSONObj& key) const { + invariant(isSharded()); + return _cm->getShardKeyPattern().isShardKey(key); + } + + const BSONObj& getKeyPattern() const { + invariant(isSharded()); + return _cm->getShardKeyPattern().toBSON(); + } + + const std::vector>& getKeyPatternFields() const { + invariant(isSharded()); + return _cm->getShardKeyPattern().getKeyPatternFields(); + } + + BSONObj getMinKey() const { + invariant(isSharded()); + return _cm->getShardKeyPattern().getKeyPattern().globalMin(); + } + + BSONObj getMaxKey() const { + invariant(isSharded()); + return _cm->getShardKeyPattern().getKeyPattern().globalMax(); + } + + bool uuidMatches(UUID uuid) const { + invariant(isSharded()); + return _cm->uuidMatches(uuid); + } + /** * Returns just the shard key fields, if the collection is sharded, and the _id field, from * `doc`. Does not alter any field values (e.g. by hashing); values are copied verbatim. @@ -95,30 +135,18 @@ public: */ void toBSONBasic(BSONObjBuilder& bb) const; - /** - * BSON output of the chunks metadata into a BSONArray - */ - void toBSONChunks(BSONArrayBuilder& bb) const; - /** * String output of the collection and shard versions. */ std::string toStringBasic() const; - /** - * Obtains the shard id with which this collection metadata is configured. - */ - const ShardId& shardId() const { - invariant(isSharded()); - return _thisShardId; - } + // + // Methods used for orphan filtering and general introspection of the chunks owned by the shard + // - /** - * Returns true if 'key' contains exactly the same fields as the shard key pattern. - */ - bool isValidKey(const BSONObj& key) const { + ChunkManager* getChunkManager() const { invariant(isSharded()); - return _cm->getShardKeyPattern().isShardKey(key); + return _cm.get(); } /** @@ -146,7 +174,7 @@ public: /** * Returns true if the argument range overlaps any chunk. */ - bool rangeOverlapsChunk(ChunkRange const& range) const { + bool rangeOverlapsChunk(const ChunkRange& range) const { invariant(isSharded()); return _cm->rangeOverlapsShard(range, _thisShardId); } @@ -169,49 +197,25 @@ public: * * @return orphanRange the output range. Note that the NS is not set. */ - boost::optional getNextOrphanRange(RangeMap const& receiveMap, - BSONObj const& lookupKey) const; + boost::optional getNextOrphanRange(const RangeMap& receiveMap, + const BSONObj& lookupKey) const; /** * Returns all the chunks which are contained on this shard. */ RangeMap getChunks() const; - const BSONObj& getKeyPattern() const { - invariant(isSharded()); - return _cm->getShardKeyPattern().toBSON(); - } - - const std::vector>& getKeyPatternFields() const { - invariant(isSharded()); - return _cm->getShardKeyPattern().getKeyPatternFields(); - } - - BSONObj getMinKey() const { - invariant(isSharded()); - return _cm->getShardKeyPattern().getKeyPattern().globalMin(); - } - - BSONObj getMaxKey() const { - invariant(isSharded()); - return _cm->getShardKeyPattern().getKeyPattern().globalMax(); - } - - std::shared_ptr getChunkManager() const { - invariant(isSharded()); - return _cm; - } - - bool uuidMatches(UUID uuid) const { - invariant(isSharded()); - return _cm->uuidMatches(uuid); - } + /** + * BSON output of the chunks metadata into a BSONArray + */ + void toBSONChunks(BSONArrayBuilder* builder) const; private: - // The full routing table for the collection. + // The full routing table for the collection or nullptr if the collection is not sharded std::shared_ptr _cm; - // The identity of this shard, for the purpose of answering "key belongs to me" queries. + // The identity of this shard, for the purpose of answering "key belongs to me" queries. If the + // collection is not sharded (_cm is nullptr), then this value will be empty. ShardId _thisShardId; }; diff --git a/src/mongo/db/s/collection_metadata_filtering_test.cpp b/src/mongo/db/s/collection_metadata_filtering_test.cpp index 25ef8f116ba..6463636c22a 100644 --- a/src/mongo/db/s/collection_metadata_filtering_test.cpp +++ b/src/mongo/db/s/collection_metadata_filtering_test.cpp @@ -140,7 +140,7 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsInTheFuture) { AutoGetCollection autoColl(operationContext(), kNss, MODE_IS); auto* const css = CollectionShardingState::get(operationContext(), kNss); - testFn(css->getMetadataForOperation(operationContext())); + testFn(css->getOrphansFilter(operationContext())); } { @@ -171,7 +171,7 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsInThePast) { AutoGetCollection autoColl(operationContext(), kNss, MODE_IS); auto* const css = CollectionShardingState::get(operationContext(), kNss); - testFn(css->getMetadataForOperation(operationContext())); + testFn(css->getOrphansFilter(operationContext())); } { @@ -210,7 +210,7 @@ TEST_F(CollectionMetadataFilteringTest, FilterDocumentsTooFarInThePastThrowsStal AutoGetCollection autoColl(operationContext(), kNss, MODE_IS); auto* const css = CollectionShardingState::get(operationContext(), kNss); - testFn(css->getMetadataForOperation(operationContext())); + testFn(css->getOrphansFilter(operationContext())); } { diff --git a/src/mongo/db/s/collection_sharding_state.cpp b/src/mongo/db/s/collection_sharding_state.cpp index 980c7116469..b9037015042 100644 --- a/src/mongo/db/s/collection_sharding_state.cpp +++ b/src/mongo/db/s/collection_sharding_state.cpp @@ -110,7 +110,8 @@ private: const auto kUnshardedCollection = std::make_shared(); -ChunkVersion getOperationReceivedVersion(OperationContext* opCtx, const NamespaceString& nss) { +boost::optional getOperationReceivedVersion(OperationContext* opCtx, + const NamespaceString& nss) { auto& oss = OperationShardingState::get(opCtx); // If there is a version attached to the OperationContext, use it as the received version, @@ -124,12 +125,12 @@ ChunkVersion getOperationReceivedVersion(OperationContext* opCtx, const Namespac // in a single call, the lack of version for a namespace on the collection must be treated // as UNSHARDED return connectionShardVersion.value_or(ChunkVersion::UNSHARDED()); - } else { - // There is no shard version information on either 'opCtx' or 'client'. This means that the - // operation represented by 'opCtx' is unversioned, and the shard version is always OK for - // unversioned operations. - return ChunkVersion::IGNORED(); } + + // There is no shard version information on either 'opCtx' or 'client'. This means that the + // operation represented by 'opCtx' is unversioned, and the shard version is always OK for + // unversioned operations + return boost::none; } } // namespace @@ -151,12 +152,10 @@ void CollectionShardingState::report(OperationContext* opCtx, BSONObjBuilder* bu collectionsMap->report(opCtx, builder); } -ScopedCollectionMetadata CollectionShardingState::getMetadataForOperation(OperationContext* opCtx) { +ScopedCollectionMetadata CollectionShardingState::getOrphansFilter(OperationContext* opCtx) { const auto receivedShardVersion = getOperationReceivedVersion(opCtx, _nss); - - if (ChunkVersion::isIgnoredVersion(receivedShardVersion)) { + if (!receivedShardVersion) return {kUnshardedCollection}; - } const auto atClusterTime = repl::ReadConcernArgs::get(opCtx).getArgsAtClusterTime(); auto optMetadata = _getMetadata(atClusterTime); @@ -193,8 +192,12 @@ boost::optional CollectionShardingState::getCurrentShardVersionIfK } void CollectionShardingState::checkShardVersionOrThrow(OperationContext* opCtx) { - const auto receivedShardVersion = getOperationReceivedVersion(opCtx, _nss); + const auto optReceivedShardVersion = getOperationReceivedVersion(opCtx, _nss); + + if (!optReceivedShardVersion) + return; + const auto& receivedShardVersion = *optReceivedShardVersion; if (ChunkVersion::isIgnoredVersion(receivedShardVersion)) { return; } @@ -203,7 +206,7 @@ void CollectionShardingState::checkShardVersionOrThrow(OperationContext* opCtx) invariant(repl::ReadConcernArgs::get(opCtx).getLevel() != repl::ReadConcernLevel::kAvailableReadConcern); - const auto metadata = getMetadataForOperation(opCtx); + const auto metadata = getCurrentMetadata(); const auto wantedShardVersion = metadata->isSharded() ? metadata->getShardVersion() : ChunkVersion::UNSHARDED(); diff --git a/src/mongo/db/s/collection_sharding_state.h b/src/mongo/db/s/collection_sharding_state.h index 1854fe51310..1f1a0c76e6f 100644 --- a/src/mongo/db/s/collection_sharding_state.h +++ b/src/mongo/db/s/collection_sharding_state.h @@ -75,16 +75,26 @@ public: static void report(OperationContext* opCtx, BSONObjBuilder* builder); /** - * Returns the chunk filtering metadata that the current operation should be using for that - * collection or otherwise throws if it has not been loaded yet. If the operation does not - * require a specific shard version, returns an UNSHARDED metadata. The returned object is safe - * to access outside of collection lock. + * Returns the orphan chunk filtering metadata that the current operation should be using for + * the collection. * - * If the operation context contains an 'atClusterTime' property, the returned filtering - * metadata will be tied to a specific point in time. Otherwise it will reference the latest - * time available. + * If the operation context contains an 'atClusterTime', the returned filtering metadata will be + * tied to a specific point in time. Otherwise, it will reference the latest time available. If + * the operation is not associated with a shard version (refer to + * OperationShardingState::isOperationVersioned for more info on that), returns an UNSHARDED + * metadata object. + * + * The intended users of this method are callers which need to perform orphan filtering. Use + * 'getCurrentMetadata' for all other cases, where just sharding-related properties of the + * collection are necessary (e.g., isSharded or the shard key). + * + * The returned object is safe to access even after the collection lock has been dropped. + */ + ScopedCollectionMetadata getOrphansFilter(OperationContext* opCtx); + + /** + * See the comments for 'getOrphansFilter' above for more information on this method. */ - ScopedCollectionMetadata getMetadataForOperation(OperationContext* opCtx); ScopedCollectionMetadata getCurrentMetadata(); /** diff --git a/src/mongo/db/s/get_shard_version_command.cpp b/src/mongo/db/s/get_shard_version_command.cpp index d85058ec667..bededaabe3b 100644 --- a/src/mongo/db/s/get_shard_version_command.cpp +++ b/src/mongo/db/s/get_shard_version_command.cpp @@ -131,7 +131,7 @@ public: metadata->toBSONBasic(metadataBuilder); BSONArrayBuilder chunksArr(metadataBuilder.subarrayStart("chunks")); - metadata->toBSONChunks(chunksArr); + metadata->toBSONChunks(&chunksArr); chunksArr.doneFast(); BSONArrayBuilder pendingArr(metadataBuilder.subarrayStart("pending")); diff --git a/src/mongo/db/s/op_observer_sharding_impl.cpp b/src/mongo/db/s/op_observer_sharding_impl.cpp index 9a840a86fe0..6065db52954 100644 --- a/src/mongo/db/s/op_observer_sharding_impl.cpp +++ b/src/mongo/db/s/op_observer_sharding_impl.cpp @@ -37,15 +37,25 @@ namespace mongo { namespace { + const auto getIsMigrating = OperationContext::declareDecoration(); +/** + * Write operations do shard version checking, but do not perform orphan document filtering. Because + * of this, if an update operation runs as part of a 'readConcern:snapshot' transaction, it might + * get routed to a shard which no longer owns the chunk being written to. In such cases, throw a + * MigrationConflict exception to indicate that the transaction needs to be rolled-back and + * restarted. + */ void assertIntersectingChunkHasNotMoved(OperationContext* opCtx, CollectionShardingRuntime* csr, const BSONObj& doc) { - auto metadata = csr->getMetadataForOperation(opCtx); - if (!metadata->isSharded()) { + if (!repl::ReadConcernArgs::get(opCtx).getArgsAtClusterTime()) + return; + + const auto metadata = csr->getOrphansFilter(opCtx); + if (!metadata->isSharded()) return; - } // We can assume the simple collation because shard keys do not support non-simple collations. auto chunk = metadata->getChunkManager()->findIntersectingChunkWithSimpleCollation( @@ -62,7 +72,8 @@ bool isMigratingWithCSRLock(CollectionShardingRuntime* csr, auto msm = MigrationSourceManager::get(csr, csrLock); return msm && msm->getCloner()->isDocumentInMigratingChunk(docToDelete); } -} + +} // namespace bool OpObserverShardingImpl::isMigrating(OperationContext* opCtx, NamespaceString const& nss, @@ -95,9 +106,7 @@ void OpObserverShardingImpl::shardObserveInsertOp(OperationContext* opCtx, csr->checkShardVersionOrThrow(opCtx); if (inMultiDocumentTransaction) { - if (repl::ReadConcernArgs::get(opCtx).getArgsAtClusterTime()) { - assertIntersectingChunkHasNotMoved(opCtx, csr, insertedDoc); - } + assertIntersectingChunkHasNotMoved(opCtx, csr, insertedDoc); return; } @@ -118,9 +127,7 @@ void OpObserverShardingImpl::shardObserveUpdateOp(OperationContext* opCtx, csr->checkShardVersionOrThrow(opCtx); if (inMultiDocumentTransaction) { - if (repl::ReadConcernArgs::get(opCtx).getArgsAtClusterTime()) { - assertIntersectingChunkHasNotMoved(opCtx, csr, updatedDoc); - } + assertIntersectingChunkHasNotMoved(opCtx, csr, updatedDoc); return; } @@ -141,9 +148,7 @@ void OpObserverShardingImpl::shardObserveDeleteOp(OperationContext* opCtx, csr->checkShardVersionOrThrow(opCtx); if (inMultiDocumentTransaction) { - if (repl::ReadConcernArgs::get(opCtx).getArgsAtClusterTime()) { - assertIntersectingChunkHasNotMoved(opCtx, csr, documentKey); - } + assertIntersectingChunkHasNotMoved(opCtx, csr, documentKey); return; } diff --git a/src/mongo/db/s/shard_server_op_observer.cpp b/src/mongo/db/s/shard_server_op_observer.cpp index aab9e095b43..e257deaee70 100644 --- a/src/mongo/db/s/shard_server_op_observer.cpp +++ b/src/mongo/db/s/shard_server_op_observer.cpp @@ -206,7 +206,7 @@ void ShardServerOpObserver::onInserts(OperationContext* opCtx, std::vector::const_iterator end, bool fromMigrate) { auto* const css = CollectionShardingState::get(opCtx, nss); - const auto metadata = css->getMetadataForOperation(opCtx); + const auto metadata = css->getCurrentMetadata(); for (auto it = begin; it != end; ++it) { const auto& insertedDoc = it->doc; @@ -236,7 +236,7 @@ void ShardServerOpObserver::onInserts(OperationContext* opCtx, void ShardServerOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateEntryArgs& args) { auto* const css = CollectionShardingState::get(opCtx, args.nss); - const auto metadata = css->getMetadataForOperation(opCtx); + const auto metadata = css->getCurrentMetadata(); if (args.nss == NamespaceString::kShardConfigCollectionsNamespace) { // Notification of routing table changes are only needed on secondaries -- cgit v1.2.1