SERVER-63243 Range deleter must not clean up orphan ranges in a round-robin fashion

(cherry picked from commit f44581d5bfe275a3b9f0454dd7843c04ccfd1f2d)

1 files changed, 73 insertions, 62 deletions

diff --git a/src/mongo/db/s/range_deletion_util.cpp b/src/mongo/db/s/range_deletion_util.cpp
index 9a4263c4005..64b07eab270 100644
--- a/src/mongo/db/s/range_deletion_util.cpp
+++ b/src/mongo/db/s/range_deletion_util.cpp
@@ -310,69 +310,80 @@ ExecutorFuture<void> deleteRangeInBatches(const std::shared_ptr<executor::TaskEx
                                           const boost::optional<UUID>& migrationId,
                                           int numDocsToRemovePerBatch,
                                           Milliseconds delayBetweenBatches) {
-    return AsyncTry([=] {
-               return withTemporaryOperationContext(
-                   [=](OperationContext* opCtx) {
-                       LOGV2_DEBUG(5346200,
-                                   1,
-                                   "Starting batch deletion",
-                                   "namespace"_attr = nss,
-                                   "range"_attr = redact(range.toString()),
-                                   "numDocsToRemovePerBatch"_attr = numDocsToRemovePerBatch,
-                                   "delayBetweenBatches"_attr = delayBetweenBatches);
-
-                       if (migrationId) {
-                           ensureRangeDeletionTaskStillExists(opCtx, *migrationId);
-                       }
-
-                       AutoGetCollection autoColl(opCtx, nss, MODE_IX);
-                       auto* const collection = autoColl.getCollection();
-
-                       // 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, collectionUuid));
-
-                       auto numDeleted = uassertStatusOK(deleteNextBatch(
-                           opCtx, collection, keyPattern, range, numDocsToRemovePerBatch));
-
-                       LOGV2_DEBUG(
-                           23769,
-                           1,
-                           "Deleted {numDeleted} documents in pass in namespace {namespace} with "
-                           "UUID  {collectionUUID} for range {range}",
-                           "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;
-                   },
-                   nss);
-           })
-        .until([=](StatusWith<int> swNumDeleted) {
-            // Continue iterating until there are no more documents to delete, retrying on
-            // any error that doesn't indicate that this node is stepping down.
-            return (swNumDeleted.isOK() && swNumDeleted.getValue() < numDocsToRemovePerBatch) ||
-                swNumDeleted.getStatus() ==
-                ErrorCodes::RangeDeletionAbandonedBecauseCollectionWithUUIDDoesNotExist ||
-                swNumDeleted.getStatus() ==
-                ErrorCodes::RangeDeletionAbandonedBecauseTaskDocumentDoesNotExist ||
-                ErrorCodes::isShutdownError(swNumDeleted.getStatus()) ||
-                ErrorCodes::isNotPrimaryError(swNumDeleted.getStatus());
+    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) {
+                            LOGV2_DEBUG(5346200,
+                                        1,
+                                        "Starting batch deletion",
+                                        "namespace"_attr = nss,
+                                        "range"_attr = redact(range.toString()),
+                                        "numDocsToRemovePerBatch"_attr = numDocsToRemovePerBatch,
+                                        "delayBetweenBatches"_attr = delayBetweenBatches);
+
+                            if (migrationId) {
+                                ensureRangeDeletionTaskStillExists(opCtx, *migrationId);
+                            }
+
+                            AutoGetCollection autoColl(opCtx, nss, MODE_IX);
+                            auto* const collection = autoColl.getCollection();
+
+                            // 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, collectionUuid));
+
+                            auto numDeleted = uassertStatusOK(deleteNextBatch(
+                                opCtx, collection, keyPattern, range, numDocsToRemovePerBatch));
+
+                            LOGV2_DEBUG(23769,
+                                        1,
+                                        "Deleted {numDeleted} documents in pass in namespace "
+                                        "{namespace} with "
+                                        "UUID  {collectionUUID} for range {range}",
+                                        "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 ||
+                        ErrorCodes::isShutdownError(errorCode) ||
+                        ErrorCodes::isNotPrimaryError(errorCode)) {
+                        return ex.toStatus();
+                    };
+                }
+            }
+            return Status::OK();
         })
-        .on(executor)
         .ignoreValue();
 }