SERVER-67385 Range deletion tasks on primary must not be scheduled before ongoing queries finish

(cherry picked from commit 32c2f632eaa7bf80607880162ec5e4eaeb22d7fe)

3 files changed, 85 insertions, 9 deletions

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<MetadataManager>(
             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> _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<RangeDeletionTask> 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