SERVER-67779 Refactor `deleteRangeInBatches` to don't require an executor

2 files changed, 115 insertions, 103 deletions

diff --git a/src/mongo/db/s/range_deletion_util.cpp b/src/mongo/db/s/range_deletion_util.cpp
index b4ed292c257..b7dd5efa719 100644
--- a/src/mongo/db/s/range_deletion_util.cpp
+++ b/src/mongo/db/s/range_deletion_util.cpp
@@ -290,108 +290,21 @@ void markRangeDeletionTaskAsProcessing(OperationContext* opCtx, const UUID& migr
  * Delete the range in a sequence of batches until there are no more documents to delete or deletion
  * returns an error.
  */
-ExecutorFuture<void> deleteRangeInBatches(const std::shared_ptr<executor::TaskExecutor>& executor,
-                                          const NamespaceString& nss,
-                                          const UUID& collectionUuid,
-                                          const BSONObj& keyPattern,
-                                          const ChunkRange& range,
-                                          const UUID& migrationId) {
-    return ExecutorFuture<void>(executor)
-        .then([=] {
-            bool allDocsRemoved = false;
-            // Delete all batches in this range unless a stepdown error occurs. Do not yield the
-            // executor to ensure that this range is fully deleted before another range is
-            // processed.
-            while (!allDocsRemoved) {
-                try {
-                    allDocsRemoved = withTemporaryOperationContext(
-                        [=](OperationContext* opCtx) {
-                            int numDocsToRemovePerBatch = rangeDeleterBatchSize.load();
-                            if (numDocsToRemovePerBatch <= 0) {
-                                numDocsToRemovePerBatch = kRangeDeleterBatchSizeDefault;
-                            }
-
-                            Milliseconds delayBetweenBatches(rangeDeleterBatchDelayMS.load());
-
-                            LOGV2_DEBUG(5346200,
-                                        1,
-                                        "Starting batch deletion",
-                                        "namespace"_attr = nss,
-                                        "range"_attr = redact(range.toString()),
-                                        "numDocsToRemovePerBatch"_attr = numDocsToRemovePerBatch,
-                                        "delayBetweenBatches"_attr = delayBetweenBatches);
-
-                            ensureRangeDeletionTaskStillExists(opCtx, migrationId);
-
-                            int numDeleted;
-
-                            {
-                                AutoGetCollection collection(opCtx, nss, MODE_IX);
-
-                                // Ensure the collection exists and has not been dropped or dropped
-                                // and recreated
-                                uassert(
-                                    ErrorCodes::
-                                        RangeDeletionAbandonedBecauseCollectionWithUUIDDoesNotExist,
-                                    "Collection has been dropped since enqueuing this range "
-                                    "deletion task. No need to delete documents.",
-                                    !collectionUuidHasChanged(
-                                        nss, collection.getCollection(), collectionUuid));
-
-                                markRangeDeletionTaskAsProcessing(opCtx, migrationId);
-
-                                numDeleted =
-                                    uassertStatusOK(deleteNextBatch(opCtx,
-                                                                    collection.getCollection(),
-                                                                    keyPattern,
-                                                                    range,
-                                                                    numDocsToRemovePerBatch));
-
-                                migrationutil::persistUpdatedNumOrphans(
-                                    opCtx, migrationId, collectionUuid, -numDeleted);
-
-                                if (MONGO_unlikely(hangAfterDoingDeletion.shouldFail())) {
-                                    hangAfterDoingDeletion.pauseWhileSet(opCtx);
-                                }
-                            }
-
-                            LOGV2_DEBUG(23769,
-                                        1,
-                                        "Deleted documents in pass",
-                                        "numDeleted"_attr = numDeleted,
-                                        "namespace"_attr = nss.ns(),
-                                        "collectionUUID"_attr = collectionUuid,
-                                        "range"_attr = range.toString());
-
-                            if (numDeleted > 0) {
-                                // (SERVER-62368) The range-deleter executor is mono-threaded, so
-                                // sleeping synchronously for `delayBetweenBatches` ensures that no
-                                // other batch is going to be cleared up before the expected delay.
-                                opCtx->sleepFor(delayBetweenBatches);
-                            }
-
-                            return numDeleted < numDocsToRemovePerBatch;
-                        },
-                        nss);
-                } catch (const DBException& ex) {
-                    // Errors other than those indicating stepdown and those that indicate that the
-                    // range deletion can no longer occur should be retried.
-                    auto errorCode = ex.code();
-                    if (errorCode ==
-                            ErrorCodes::
-                                RangeDeletionAbandonedBecauseCollectionWithUUIDDoesNotExist ||
-                        errorCode ==
-                            ErrorCodes::RangeDeletionAbandonedBecauseTaskDocumentDoesNotExist ||
-                        errorCode == ErrorCodes::KeyPatternShorterThanBound ||
-                        ErrorCodes::isShutdownError(errorCode) ||
-                        ErrorCodes::isNotPrimaryError(errorCode)) {
-                        return ex.toStatus();
-                    };
-                }
-            }
-            return Status::OK();
-        })
-        .ignoreValue();
+ExecutorFuture<void> deleteRangeInBatchesWithExecutor(
+    const std::shared_ptr<executor::TaskExecutor>& executor,
+    const NamespaceString& nss,
+    const UUID& collectionUuid,
+    const BSONObj& keyPattern,
+    const ChunkRange& range,
+    const UUID& migrationId) {
+    return ExecutorFuture<void>(executor).then([=] {
+        return withTemporaryOperationContext(
+            [=](OperationContext* opCtx) {
+                return deleteRangeInBatches(
+                    opCtx, nss, collectionUuid, keyPattern, range, migrationId);
+            },
+            nss);
+    });
 }
 
 void removePersistentRangeDeletionTask(const NamespaceString& nss, UUID migrationId) {
@@ -447,6 +360,92 @@ std::vector<RangeDeletionTask> getPersistentRangeDeletionTasks(OperationContext*
 
 }  // namespace
 
+Status deleteRangeInBatches(OperationContext* opCtx,
+                            const NamespaceString& nss,
+                            const UUID& collectionUuid,
+                            const BSONObj& keyPattern,
+                            const ChunkRange& range,
+                            const UUID& migrationId) {
+    bool allDocsRemoved = false;
+    // Delete all batches in this range unless a stepdown error occurs. Do not yield the
+    // executor to ensure that this range is fully deleted before another range is
+    // processed.
+    while (!allDocsRemoved) {
+        try {
+            int numDocsToRemovePerBatch = rangeDeleterBatchSize.load();
+            if (numDocsToRemovePerBatch <= 0) {
+                numDocsToRemovePerBatch = kRangeDeleterBatchSizeDefault;
+            }
+
+            Milliseconds delayBetweenBatches(rangeDeleterBatchDelayMS.load());
+
+            LOGV2_DEBUG(5346200,
+                        1,
+                        "Starting batch deletion",
+                        "namespace"_attr = nss,
+                        "range"_attr = redact(range.toString()),
+                        "numDocsToRemovePerBatch"_attr = numDocsToRemovePerBatch,
+                        "delayBetweenBatches"_attr = delayBetweenBatches);
+
+            ensureRangeDeletionTaskStillExists(opCtx, migrationId);
+
+            int numDeleted;
+            {
+                AutoGetCollection collection(opCtx, nss, MODE_IX);
+
+                // Ensure the collection exists and has not been dropped or dropped
+                // and recreated
+                uassert(ErrorCodes::RangeDeletionAbandonedBecauseCollectionWithUUIDDoesNotExist,
+                        "Collection has been dropped since enqueuing this range "
+                        "deletion task. No need to delete documents.",
+                        !collectionUuidHasChanged(nss, collection.getCollection(), collectionUuid));
+
+                markRangeDeletionTaskAsProcessing(opCtx, migrationId);
+
+                numDeleted = uassertStatusOK(deleteNextBatch(
+                    opCtx, collection.getCollection(), keyPattern, range, numDocsToRemovePerBatch));
+
+                migrationutil::persistUpdatedNumOrphans(
+                    opCtx, migrationId, collectionUuid, -numDeleted);
+
+                if (MONGO_unlikely(hangAfterDoingDeletion.shouldFail())) {
+                    hangAfterDoingDeletion.pauseWhileSet(opCtx);
+                }
+            }
+
+            LOGV2_DEBUG(23769,
+                        1,
+                        "Deleted documents in pass",
+                        "numDeleted"_attr = numDeleted,
+                        "namespace"_attr = nss.ns(),
+                        "collectionUUID"_attr = collectionUuid,
+                        "range"_attr = range.toString());
+
+            if (numDeleted > 0) {
+                // (SERVER-62368) The range-deleter executor is mono-threaded, so
+                // sleeping synchronously for `delayBetweenBatches` ensures that no
+                // other batch is going to be cleared up before the expected delay.
+                opCtx->sleepFor(delayBetweenBatches);
+            }
+
+            allDocsRemoved = numDeleted < numDocsToRemovePerBatch;
+        } catch (const DBException& ex) {
+            // Errors other than those indicating stepdown and those that indicate that the
+            // range deletion can no longer occur should be retried.
+            auto errorCode = ex.code();
+            if (errorCode ==
+                    ErrorCodes::RangeDeletionAbandonedBecauseCollectionWithUUIDDoesNotExist ||
+                errorCode == ErrorCodes::RangeDeletionAbandonedBecauseTaskDocumentDoesNotExist ||
+                errorCode == ErrorCodes::KeyPatternShorterThanBound ||
+                ErrorCodes::isShutdownError(errorCode) ||
+                ErrorCodes::isNotPrimaryError(errorCode)) {
+                return ex.toStatus();
+            };
+        }
+    }
+    return Status::OK();
+}
+
 void snapshotRangeDeletionsForRename(OperationContext* opCtx,
                                      const NamespaceString& fromNss,
                                      const NamespaceString& toNss) {
@@ -527,7 +526,7 @@ SharedSemiFuture<void> removeDocumentsInRange(
                         "namespace"_attr = nss.ns(),
                         "range"_attr = redact(range.toString()));
 
-            return deleteRangeInBatches(
+            return deleteRangeInBatchesWithExecutor(
                        executor, nss, collectionUuid, keyPattern, range, migrationId)
                 .onCompletion([=](Status s) {
                     if (!s.isOK() &&
diff --git a/src/mongo/db/s/range_deletion_util.h b/src/mongo/db/s/range_deletion_util.h
index aaf6bfae75f..9569f7f5607 100644
--- a/src/mongo/db/s/range_deletion_util.h
+++ b/src/mongo/db/s/range_deletion_util.h
@@ -40,6 +40,8 @@
 namespace mongo {
 
 /**
+ * DO NOT USE - only necessary for the legacy range deleter
+ *
  * Deletes a range of orphaned documents for the given namespace and collection UUID. Returns a
  * future which will be resolved when the range has finished being deleted. The resulting future
  * will contain an error in cases where the range could not be deleted successfully.
@@ -63,6 +65,17 @@ SharedSemiFuture<void> removeDocumentsInRange(
     Seconds delayForActiveQueriesOnSecondariesToComplete);
 
 /**
+ * Delete the range in a sequence of batches until there are no more documents to delete or deletion
+ * returns an error.
+ */
+Status deleteRangeInBatches(OperationContext* opCtx,
+                            const NamespaceString& nss,
+                            const UUID& collectionUuid,
+                            const BSONObj& keyPattern,
+                            const ChunkRange& range,
+                            const UUID& migrationId);
+
+/**
  * - Retrieves source collection's persistent range deletion tasks from `config.rangeDeletions`
  * - Associates tasks to the target collection
  * - Stores tasks in `config.rangeDeletionsForRename`