From 834bbf20f9af79970d018594bb50ce9d98c023fb Mon Sep 17 00:00:00 2001 From: Pierlauro Sciarelli Date: Fri, 12 Aug 2022 11:03:48 +0000 Subject: SERVER-67385 Range deletion tasks on primary must not be scheduled before ongoing queries finish (cherry picked from commit 32c2f632eaa7bf80607880162ec5e4eaeb22d7fe) --- src/mongo/db/s/collection_sharding_runtime.cpp | 13 ++--- src/mongo/db/s/collection_sharding_runtime.h | 22 ++++++-- .../db/s/collection_sharding_runtime_test.cpp | 59 ++++++++++++++++++++++ 3 files changed, 85 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/mongo/db/s/collection_sharding_runtime.cpp b/src/mongo/db/s/collection_sharding_runtime.cpp index 4960ed1166c..a66fd675743 100644 --- a/src/mongo/db/s/collection_sharding_runtime.cpp +++ b/src/mongo/db/s/collection_sharding_runtime.cpp @@ -209,9 +209,11 @@ void CollectionShardingRuntime::setFilteringMetadata_withLock(OperationContext* _metadataType = MetadataType::kUnsharded; _metadataManager.reset(); ++_numMetadataManagerChanges; - } else if (!_metadataManager || - !newMetadata.uuidMatches(_metadataManager->getCollectionUuid())) { - _metadataType = MetadataType::kSharded; + return; + } + + _metadataType = MetadataType::kSharded; + if (!_metadataManager || !newMetadata.uuidMatches(_metadataManager->getCollectionUuid())) { _metadataManager = std::make_shared( opCtx->getServiceContext(), _nss, _rangeDeleterExecutor, newMetadata); ++_numMetadataManagerChanges; @@ -234,7 +236,6 @@ void CollectionShardingRuntime::clearFilteringMetadata(OperationContext* opCtx) "Clearing collection metadata", "namespace"_attr = _nss); _metadataType = MetadataType::kUnknown; - _metadataManager.reset(); } } @@ -264,7 +265,7 @@ Status CollectionShardingRuntime::waitForClean(OperationContext* opCtx, // If the metadata was reset, or the collection was dropped and recreated since the // metadata manager was created, return an error. - if (!self->_metadataManager || + if (self->_metadataType != MetadataType::kSharded || (collectionUuid != self->_metadataManager->getCollectionUuid())) { return {ErrorCodes::ConflictingOperationInProgress, "Collection being migrated was dropped and created or otherwise had its " @@ -406,7 +407,7 @@ void CollectionShardingRuntime::appendShardVersion(BSONObjBuilder* builder) { size_t CollectionShardingRuntime::numberOfRangesScheduledForDeletion() const { stdx::lock_guard lk(_metadataManagerLock); - if (_metadataManager) { + if (_metadataType == MetadataType::kSharded) { return _metadataManager->numberOfRangesScheduledForDeletion(); } return 0; diff --git a/src/mongo/db/s/collection_sharding_runtime.h b/src/mongo/db/s/collection_sharding_runtime.h index 6af24245375..bbc343d5bcb 100644 --- a/src/mongo/db/s/collection_sharding_runtime.h +++ b/src/mongo/db/s/collection_sharding_runtime.h @@ -279,10 +279,26 @@ private: // Tracks whether the filtering metadata is unknown, unsharded, or sharded enum class MetadataType { kUnknown, kUnsharded, kSharded } _metadataType; - // If the collection is sharded, contains all the metadata associated with this collection. + // If the collection state is known and is unsharded, this will be nullptr. // - // If the collection is unsharded, the metadata has not been set yet, or the metadata has been - // specifically reset by calling clearFilteringMetadata(), this will be nullptr; + // If the collection state is known and is sharded, this will point to the metadata associated + // with this collection. + // + // If the collection state is unknown: + // - If the metadata had never been set yet, this will be nullptr. + // - If the collection state was known and was sharded, this contains the metadata that + // were known for the collection before the last invocation of clearFilteringMetadata(). + // + // The following matrix enumerates the valid (Y) and invalid (X) scenarios. + // _________________________________ + // | _metadataType (collection state)| + // |_________________________________| + // | UNKNOWN | UNSHARDED | SHARDED | + // _______________________|_________|___________|___________| + // |_metadataManager unset | Y | Y | X | + // |_______________________|_________|___________|___________| + // |_metadataManager set | Y | X | Y | + // |_______________________|_________|___________|___________| std::shared_ptr _metadataManager; // Used for testing to check the number of times a new MetadataManager has been installed. diff --git a/src/mongo/db/s/collection_sharding_runtime_test.cpp b/src/mongo/db/s/collection_sharding_runtime_test.cpp index fbac57d611a..d1c9457cb89 100644 --- a/src/mongo/db/s/collection_sharding_runtime_test.cpp +++ b/src/mongo/db/s/collection_sharding_runtime_test.cpp @@ -34,9 +34,11 @@ #include "mongo/db/catalog_raii.h" #include "mongo/db/db_raii.h" #include "mongo/db/dbdirectclient.h" +#include "mongo/db/persistent_task_store.h" #include "mongo/db/repl/wait_for_majority_service.h" #include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/operation_sharding_state.h" +#include "mongo/db/s/range_deletion_task_gen.h" #include "mongo/db/s/shard_filtering_metadata_refresh.h" #include "mongo/db/s/shard_server_test_fixture.h" #include "mongo/db/s/sharding_state.h" @@ -547,5 +549,62 @@ TEST_F(CollectionShardingRuntimeWithRangeDeleterTest, ASSERT(cleanupComplete.isReady()); } +TEST_F(CollectionShardingRuntimeWithRangeDeleterTest, + WaitForCleanCorrectEvenAfterClearFollowedBySetFilteringMetadata) { + globalFailPointRegistry().find("suspendRangeDeletion")->setMode(FailPoint::alwaysOn); + ON_BLOCK_EXIT( + [&] { globalFailPointRegistry().find("suspendRangeDeletion")->setMode(FailPoint::off); }); + + auto insertRangeDeletionTask = [&](OperationContext* opCtx, + const NamespaceString& nss, + const UUID& uuid, + const ChunkRange& range, + int64_t numOrphans) -> RangeDeletionTask { + PersistentTaskStore store(NamespaceString::kRangeDeletionNamespace); + auto migrationId = UUID::gen(); + RangeDeletionTask t( + migrationId, nss, uuid, ShardId("donor"), range, CleanWhenEnum::kDelayed); + t.setPending(true); + store.add(opCtx, t); + + auto query = BSON(RangeDeletionTask::kIdFieldName << migrationId); + t.setPending(boost::none); + auto update = t.toBSON(); + store.update(opCtx, query, update); + + return t; + }; + + OperationContext* opCtx = operationContext(); + auto metadata = makeShardedMetadata(opCtx, uuid()); + csr().setFilteringMetadata(opCtx, metadata); + const ChunkRange range = ChunkRange(BSON(kShardKey << MINKEY), BSON(kShardKey << MAXKEY)); + const auto task = insertRangeDeletionTask(opCtx, kTestNss, uuid(), range, 0); + + // Schedule range deletion that will hang due to `suspendRangeDeletion` failpoint + auto cleanupComplete = + csr().cleanUpRange(range, task.getId(), CollectionShardingRuntime::CleanWhen::kNow); + + // Clear and set again filtering metadata + csr().clearFilteringMetadata(opCtx); + csr().setFilteringMetadata(opCtx, metadata); + + auto waitForCleanUp = [&](Milliseconds timeout) { + return CollectionShardingRuntime::waitForClean(opCtx, kTestNss, uuid(), range, timeout); + }; + + // Check that the hanging range deletion is still tracked even following a clear of the metadata + auto status = waitForCleanUp(Milliseconds(100)); + ASSERT_NOT_OK(status); + ASSERT(!cleanupComplete.isReady()); + + globalFailPointRegistry().find("suspendRangeDeletion")->setMode(FailPoint::off); + + // Check that the range deletion is not tracked anymore after it succeeds + status = waitForCleanUp(Milliseconds::max()); + ASSERT_OK(status); + ASSERT(cleanupComplete.isReady()); +} + } // namespace } // namespace mongo -- cgit v1.2.1