SERVER-38050 Validate the sameness of collection metadata after the range deleter's deletion loop

(cherry picked from commit cee9c4deed8bbf0c612b465be4625d5d0775d204)

3 files changed, 74 insertions, 35 deletions

diff --git a/src/mongo/db/s/collection_range_deleter.cpp b/src/mongo/db/s/collection_range_deleter.cpp
index 95ae8282374..c86d8630988 100644
--- a/src/mongo/db/s/collection_range_deleter.cpp
+++ b/src/mongo/db/s/collection_range_deleter.cpp
@@ -131,38 +131,21 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
     {
         UninterruptibleLockGuard noInterrupt(opCtx->lockState());
         AutoGetCollection autoColl(opCtx, nss, MODE_IX);
-
         auto* const collection = autoColl.getCollection();
-        auto* const css = CollectionShardingRuntime::get(opCtx, nss);
-        auto& metadataManager = css->_metadataManager;
-        auto* const self = forTestOnly ? forTestOnly : &metadataManager->_rangesToClean;
-
-        const auto scopedCollectionMetadata =
-            metadataManager->getActiveMetadata(metadataManager, boost::none);
-
-        if (!forTestOnly && (!collection || !scopedCollectionMetadata->isSharded())) {
-            if (!collection) {
-                LOG(0) << "Abandoning any range deletions left over from dropped " << nss.ns();
-            } else {
-                LOG(0) << "Abandoning any range deletions left over from previously sharded"
-                       << nss.ns();
-            }
+        auto* const csr = CollectionShardingRuntime::get(opCtx, nss);
+        auto& metadataManager = csr->_metadataManager;
 
-            stdx::lock_guard<stdx::mutex> lk(css->_metadataManager->_managerLock);
-            css->_metadataManager->_clearAllCleanups(lk);
+        if (!_checkCollectionMetadataStillValid(
+                opCtx, nss, epoch, forTestOnly, collection, metadataManager)) {
             return boost::none;
         }
 
-        if (!forTestOnly && scopedCollectionMetadata->getCollVersion().epoch() != epoch) {
-            LOG(1) << "Range deletion task for " << nss.ns() << " epoch " << epoch << " woke;"
-                   << " (current is " << scopedCollectionMetadata->getCollVersion() << ")";
-            return boost::none;
-        }
+        auto* const self = forTestOnly ? forTestOnly : &metadataManager->_rangesToClean;
 
         bool writeOpLog = false;
 
         {
-            stdx::lock_guard<stdx::mutex> scopedLock(css->_metadataManager->_managerLock);
+            stdx::lock_guard<stdx::mutex> scopedLock(csr->_metadataManager->_managerLock);
             if (self->isEmpty()) {
                 LOG(1) << "No further range deletions scheduled on " << nss.ns();
                 return boost::none;
@@ -215,8 +198,8 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
                                      << "max"
                                      << range->getMax()));
             } catch (const DBException& e) {
-                stdx::lock_guard<stdx::mutex> scopedLock(css->_metadataManager->_managerLock);
-                css->_metadataManager->_clearAllCleanups(
+                stdx::lock_guard<stdx::mutex> scopedLock(csr->_metadataManager->_managerLock);
+                csr->_metadataManager->_clearAllCleanups(
                     scopedLock,
                     e.toStatus("cannot push startRangeDeletion record to Op Log,"
                                " abandoning scheduled range deletions"));
@@ -224,6 +207,9 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
             }
         }
 
+        const auto scopedCollectionMetadata =
+            metadataManager->getActiveMetadata(metadataManager, boost::none);
+
         try {
             wrote = self->_doDeletion(
                 opCtx, collection, scopedCollectionMetadata->getKeyPattern(), *range, maxToDelete);
@@ -250,7 +236,7 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
         const auto clientOpTime = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp();
 
         // Wait for replication outside the lock
-        const auto status = [&] {
+        const auto replicationStatus = [&] {
             try {
                 WriteConcernResult unusedWCResult;
                 return waitForWriteConcern(
@@ -264,15 +250,22 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
         // Don't allow lock interrupts while cleaning up.
         UninterruptibleLockGuard noInterrupt(opCtx->lockState());
         AutoGetCollection autoColl(opCtx, nss, MODE_IX);
-        auto* const css = CollectionShardingRuntime::get(opCtx, nss);
-        auto& metadataManager = css->_metadataManager;
+        auto* const collection = autoColl.getCollection();
+        auto* const csr = CollectionShardingRuntime::get(opCtx, nss);
+        auto& metadataManager = csr->_metadataManager;
+
+        if (!_checkCollectionMetadataStillValid(
+                opCtx, nss, epoch, forTestOnly, collection, metadataManager)) {
+            return boost::none;
+        }
+
         auto* const self = forTestOnly ? forTestOnly : &metadataManager->_rangesToClean;
 
-        stdx::lock_guard<stdx::mutex> scopedLock(css->_metadataManager->_managerLock);
+        stdx::lock_guard<stdx::mutex> scopedLock(csr->_metadataManager->_managerLock);
 
-        if (!status.isOK()) {
+        if (!replicationStatus.isOK()) {
             LOG(0) << "Error when waiting for write concern after removing " << nss << " range "
-                   << redact(range->toString()) << " : " << redact(status.reason());
+                   << redact(range->toString()) << " : " << redact(replicationStatus.reason());
 
             // If range were already popped (e.g. by dropping nss during the waitForWriteConcern
             // above) its notification would have been triggered, so this check suffices to ensure
@@ -281,7 +274,7 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
                 invariant(!self->isEmpty() && self->_orphans.front().notification == notification);
                 LOG(0) << "Abandoning deletion of latest range in " << nss.ns() << " after local "
                        << "deletions because of replication failure";
-                self->_pop(status);
+                self->_pop(replicationStatus);
             }
         } else {
             LOG(0) << "Finished deleting documents in " << nss.ns() << " range "
@@ -306,6 +299,39 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
     return Date_t::now() + stdx::chrono::milliseconds{rangeDeleterBatchDelayMS.load()};
 }
 
+bool CollectionRangeDeleter::_checkCollectionMetadataStillValid(
+    OperationContext* opCtx,
+    const NamespaceString& nss,
+    OID const& epoch,
+    CollectionRangeDeleter* forTestOnly,
+    Collection* collection,
+    std::shared_ptr<MetadataManager> metadataManager) {
+
+    const auto scopedCollectionMetadata =
+        metadataManager->getActiveMetadata(metadataManager, boost::none);
+
+    if (!forTestOnly && (!collection || !scopedCollectionMetadata->isSharded())) {
+        if (!collection) {
+            LOG(0) << "Abandoning any range deletions left over from dropped " << nss.ns();
+        } else {
+            LOG(0) << "Abandoning any range deletions left over from previously sharded"
+                   << nss.ns();
+        }
+
+        stdx::lock_guard<stdx::mutex> lk(metadataManager->_managerLock);
+        metadataManager->_clearAllCleanups(lk);
+        return false;
+    }
+
+    if (!forTestOnly && scopedCollectionMetadata->getCollVersion().epoch() != epoch) {
+        LOG(1) << "Range deletion task for " << nss.ns() << " epoch " << epoch << " woke;"
+               << " (current is " << scopedCollectionMetadata->getCollVersion() << ")";
+        return false;
+    }
+
+    return true;
+}
+
 StatusWith<int> CollectionRangeDeleter::_doDeletion(OperationContext* opCtx,
                                                     Collection* collection,
                                                     BSONObj const& keyPattern,
diff --git a/src/mongo/db/s/collection_range_deleter.h b/src/mongo/db/s/collection_range_deleter.h
index 4949c1febde..f6b9f161a47 100644
--- a/src/mongo/db/s/collection_range_deleter.h
+++ b/src/mongo/db/s/collection_range_deleter.h
@@ -42,6 +42,7 @@ namespace mongo {
 
 class BSONObj;
 class Collection;
+class MetadataManager;
 class OperationContext;
 
 // The maximum number of documents to delete in a single batch during range deletion.
@@ -185,6 +186,18 @@ public:
 
 private:
     /**
+     * Verifies that the metadata for the collection to be cleaned up is still valid. Makes sure
+     * the collection has not been dropped (or dropped then recreated).
+     */
+    static bool _checkCollectionMetadataStillValid(
+        OperationContext* opCtx,
+        const NamespaceString& nss,
+        OID const& epoch,
+        CollectionRangeDeleter* forTestOnly,
+        Collection* collection,
+        std::shared_ptr<MetadataManager> metadataManager);
+
+    /**
      * Performs the deletion of up to maxToDelete entries within the range in progress. Must be
      * called under the collection lock.
      *
diff --git a/src/mongo/db/s/metadata_manager.h b/src/mongo/db/s/metadata_manager.h
index 334ca1b97d0..b28bd62c6cc 100644
--- a/src/mongo/db/s/metadata_manager.h
+++ b/src/mongo/db/s/metadata_manager.h
@@ -138,12 +138,12 @@ public:
     boost::optional<ChunkRange> getNextOrphanRange(BSONObj const& from) const;
 
 private:
+    // For access to _managerLock, _rangesToClean, and _clearAllCleanups under task callback
+    friend class CollectionRangeDeleter;
+
     // Management of the _metadata list is implemented in RangePreserver
     friend class RangePreserver;
 
-    // For access to _rangesToClean and _managerLock under task callback
-    friend boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange(
-        OperationContext*, NamespaceString const&, OID const&, int, CollectionRangeDeleter*);
 
     /**
      * Represents an instance of what the filtering metadata for this collection was at a particular