SERVER-63129 Tenant collection cloner resume should ignore “view already exists” errors while creating collections.r5.3.0-rc2

(cherry picked from commit e840bb65779035e3f5e7d1fb9b6951c291957a74)

7 files changed, 91 insertions, 55 deletions

diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript
index 17a173e2a51..53ebd0ee51e 100644
--- a/src/mongo/db/catalog/SConscript
+++ b/src/mongo/db/catalog/SConscript
@@ -309,6 +309,7 @@ env.Library(
     ],
     LIBDEPS_PRIVATE=[
         '$BUILD_DIR/mongo/db/concurrency/lock_manager',
+        '$BUILD_DIR/mongo/db/views/views',
         'collection',
         'collection_catalog',
     ],
@@ -499,6 +500,9 @@ env.Library(
         'rename_collection.cpp',
         'list_indexes.cpp',
     ],
+    LIBDEPS=[
+        'collection_catalog_helper',
+    ],
     LIBDEPS_PRIVATE=[
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/db/db_raii',
diff --git a/src/mongo/db/catalog/collection_catalog_helper.cpp b/src/mongo/db/catalog/collection_catalog_helper.cpp
index 50c78212e58..6c7dd9e19bc 100644
--- a/src/mongo/db/catalog/collection_catalog_helper.cpp
+++ b/src/mongo/db/catalog/collection_catalog_helper.cpp
@@ -31,6 +31,7 @@
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/collection_catalog.h"
 #include "mongo/db/concurrency/d_concurrency.h"
+#include "mongo/db/views/view_catalog.h"
 
 namespace mongo {
 
@@ -38,6 +39,26 @@ MONGO_FAIL_POINT_DEFINE(hangBeforeGettingNextCollection);
 
 namespace catalog {
 
+Status checkIfNamespaceExists(OperationContext* opCtx, const NamespaceString& nss) {
+    if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, nss)) {
+        return Status(ErrorCodes::NamespaceExists,
+                      str::stream() << "Collection " << nss.ns() << " already exists.");
+    }
+
+    auto view = ViewCatalog::get(opCtx)->lookup(opCtx, nss);
+    if (!view)
+        return Status::OK();
+
+    if (view->timeseries()) {
+        return Status(ErrorCodes::NamespaceExists,
+                      str::stream() << "A timeseries collection already exists. NS: " << nss);
+    }
+
+    return Status(ErrorCodes::NamespaceExists,
+                  str::stream() << "A view already exists. NS: " << nss);
+}
+
+
 void forEachCollectionFromDb(OperationContext* opCtx,
                              const TenantDatabaseName& tenantDbName,
                              LockMode collLockMode,
diff --git a/src/mongo/db/catalog/collection_catalog_helper.h b/src/mongo/db/catalog/collection_catalog_helper.h
index f45b0f54a45..60751e6b537 100644
--- a/src/mongo/db/catalog/collection_catalog_helper.h
+++ b/src/mongo/db/catalog/collection_catalog_helper.h
@@ -42,6 +42,15 @@ class CollectionCatalogEntry;
 namespace catalog {
 
 /**
+ * Returns ErrorCodes::NamespaceExists if a collection or any type of views exists on the given
+ * namespace 'nss'. Otherwise returns Status::OK().
+ *
+ * Note: If the caller calls this method without locking the collection, then the returned result
+ * could be stale right after this call.
+ */
+Status checkIfNamespaceExists(OperationContext* opCtx, const NamespaceString& nss);
+
+/**
  * Iterates through all the collections in the given database and runs the callback function on each
  * collection. If a predicate is provided, then the callback will only be executed against the
  * collections that satisfy the predicate.
diff --git a/src/mongo/db/catalog/create_collection.cpp b/src/mongo/db/catalog/create_collection.cpp
index 89357d9280b..ae9d3d4fa04 100644
--- a/src/mongo/db/catalog/create_collection.cpp
+++ b/src/mongo/db/catalog/create_collection.cpp
@@ -39,6 +39,7 @@
 #include "mongo/bson/json.h"
 #include "mongo/db/catalog/clustered_collection_util.h"
 #include "mongo/db/catalog/collection_catalog.h"
+#include "mongo/db/catalog/collection_catalog_helper.h"
 #include "mongo/db/catalog/database_holder.h"
 #include "mongo/db/catalog/index_key_validate.h"
 #include "mongo/db/commands.h"
@@ -278,26 +279,16 @@ Status _createTimeseries(OperationContext* opCtx,
         writeConflictRetry(opCtx, "createBucketCollection", bucketsNs.ns(), [&]() -> Status {
             AutoGetDb autoDb(opCtx, bucketsNs.db(), MODE_IX);
             Lock::CollectionLock bucketsCollLock(opCtx, bucketsNs, MODE_IX);
+            auto db = autoDb.ensureDbExists(opCtx);
 
             // Check if there already exist a Collection on the namespace we will later create a
             // view on. We're not holding a Collection lock for this Collection so we may only check
             // if the pointer is null or not. The answer may also change at any point after this
             // call which is fine as we properly handle an orphaned bucket collection. This check is
             // just here to prevent it from being created in the common case.
-            if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, ns)) {
-                return Status(ErrorCodes::NamespaceExists,
-                              str::stream() << "Collection already exists. NS: " << ns);
-            }
-
-            auto db = autoDb.ensureDbExists(opCtx);
-            if (auto view = ViewCatalog::get(opCtx)->lookup(opCtx, ns); view) {
-                if (view->timeseries()) {
-                    return Status(ErrorCodes::NamespaceExists,
-                                  str::stream()
-                                      << "A timeseries collection already exists. NS: " << ns);
-                }
-                return Status(ErrorCodes::NamespaceExists,
-                              str::stream() << "A view already exists. NS: " << ns);
+            Status status = catalog::checkIfNamespaceExists(opCtx, ns);
+            if (!status.isOK()) {
+                return status;
             }
 
             if (opCtx->writesAreReplicated() &&
@@ -369,23 +360,14 @@ Status _createTimeseries(OperationContext* opCtx,
             opCtx,
             NamespaceString(ns.db(), NamespaceString::kSystemDotViewsCollectionName),
             MODE_X);
+        auto db = autoColl.ensureDbExists(opCtx);
 
         // This is a top-level handler for time-series creation name conflicts. New commands coming
         // in, or commands that generated a WriteConflict must return a NamespaceExists error here
         // on conflict.
-        if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, ns)) {
-            return Status(ErrorCodes::NamespaceExists,
-                          str::stream() << "Collection already exists. NS: " << ns);
-        }
-
-        auto db = autoColl.ensureDbExists(opCtx);
-        if (auto view = ViewCatalog::get(opCtx)->lookup(opCtx, ns)) {
-            if (view->timeseries()) {
-                return {ErrorCodes::NamespaceExists,
-                        str::stream() << "A timeseries collection already exists. NS: " << ns};
-            }
-            return {ErrorCodes::NamespaceExists,
-                    str::stream() << "A view already exists. NS: " << ns};
+        Status status = catalog::checkIfNamespaceExists(opCtx, ns);
+        if (!status.isOK()) {
+            return status;
         }
 
         if (opCtx->writesAreReplicated() &&
@@ -430,7 +412,7 @@ Status _createTimeseries(OperationContext* opCtx,
         viewOptions.pipeline = timeseries::generateViewPipeline(*options.timeseries, asArray);
 
         // Create the time-series view.
-        auto status = db->userCreateNS(opCtx, ns, viewOptions);
+        status = db->userCreateNS(opCtx, ns, viewOptions);
         if (!status.isOK()) {
             return status.withContext(str::stream() << "Failed to create view on " << bucketsNs
                                                     << " for time-series collection " << ns
@@ -451,23 +433,14 @@ Status _createCollection(OperationContext* opCtx,
     return writeConflictRetry(opCtx, "create", nss.ns(), [&] {
         AutoGetDb autoDb(opCtx, nss.db(), MODE_IX);
         Lock::CollectionLock collLock(opCtx, nss, MODE_IX);
+        auto db = autoDb.ensureDbExists(opCtx);
+
         // This is a top-level handler for collection creation name conflicts. New commands coming
         // in, or commands that generated a WriteConflict must return a NamespaceExists error here
         // on conflict.
-        if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, nss)) {
-            return Status(ErrorCodes::NamespaceExists,
-                          str::stream() << "Collection already exists. NS: " << nss);
-        }
-
-        auto db = autoDb.ensureDbExists(opCtx);
-        if (auto view = ViewCatalog::get(opCtx)->lookup(opCtx, nss); view) {
-            if (view->timeseries()) {
-                return Status(ErrorCodes::NamespaceExists,
-                              str::stream()
-                                  << "A timeseries collection already exists. NS: " << nss);
-            }
-            return Status(ErrorCodes::NamespaceExists,
-                          str::stream() << "A view already exists. NS: " << nss);
+        Status status = catalog::checkIfNamespaceExists(opCtx, nss);
+        if (!status.isOK()) {
+            return status;
         }
 
         // If the FCV has changed while executing the command to the version, where the feature flag
@@ -544,7 +517,6 @@ Status _createCollection(OperationContext* opCtx,
 
         // Even though 'collectionOptions' is passed by rvalue reference, it is not safe to move
         // because 'userCreateNS' may throw a WriteConflictException.
-        Status status = Status::OK();
         if (idIndex == boost::none || collectionOptions.clusteredIndex) {
             status = db->userCreateNS(opCtx, nss, collectionOptions, /*createIdIndex=*/false);
         } else {
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index cc2b0c251c1..2125711a336 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -47,6 +47,7 @@
 #include "mongo/db/catalog/coll_mod.h"
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/collection_catalog.h"
+#include "mongo/db/catalog/collection_catalog_helper.h"
 #include "mongo/db/catalog/database_holder.h"
 #include "mongo/db/catalog/document_validation.h"
 #include "mongo/db/catalog/index_catalog.h"
@@ -481,19 +482,28 @@ Status StorageInterfaceImpl::createCollection(OperationContext* opCtx,
         AutoGetDb databaseWriteGuard(opCtx, nss.db(), MODE_IX);
         auto db = databaseWriteGuard.ensureDbExists(opCtx);
         invariant(db);
-        if (CollectionCatalog::get(opCtx)->lookupCollectionByNamespace(opCtx, nss)) {
-            return Status(ErrorCodes::NamespaceExists,
-                          str::stream() << "Collection " << nss.ns() << " already exists.");
+
+        // Check if there already exist a Collection/view on the given namespace 'nss'. The answer
+        // may change at any point after this call as we make this call without holding the
+        // collection lock. But, it is fine as we properly handle while registering the uncommitted
+        // collection with CollectionCatalog. This check is just here to prevent it from being
+        // created in the common case.
+        Status status = mongo::catalog::checkIfNamespaceExists(opCtx, nss);
+        if (!status.isOK()) {
+            return status;
         }
+
         Lock::CollectionLock lk(opCtx, nss, MODE_IX);
         WriteUnitOfWork wuow(opCtx);
         try {
             auto coll = db->createCollection(opCtx, nss, options, createIdIndex, idIndexSpec);
             invariant(coll);
+
+            // This commit call can throw if a view already exists while registering the collection.
+            wuow.commit();
         } catch (const AssertionException& ex) {
             return ex.toStatus();
         }
-        wuow.commit();
 
         return Status::OK();
     });
diff --git a/src/mongo/db/repl/tenant_collection_cloner.cpp b/src/mongo/db/repl/tenant_collection_cloner.cpp
index 4e391644b5a..d74b0760cd4 100644
--- a/src/mongo/db/repl/tenant_collection_cloner.cpp
+++ b/src/mongo/db/repl/tenant_collection_cloner.cpp
@@ -394,17 +394,24 @@ BaseCloner::AfterStageBehavior TenantCollectionCloner::createCollectionStage() {
                                                     !_idIndexSpec.isEmpty() /* createIdIndex */,
                                                     _idIndexSpec);
         if (status == ErrorCodes::NamespaceExists && getSharedData()->isResuming()) {
-            // If we are resuming from a recipient failover and we have a collection on disk with
-            // the same namespace but a different uuid, it means this collection must have been
-            // dropped and re-created under a different uuid on the donor during the recipient
-            // failover. And the drop and the re-create will be covered by the oplog application
-            // phase.
+            // If we are resuming from a recipient failover we can get ErrorCodes::NamespaceExists
+            // due to following conditions:
+            //
+            // 1) We have a collection on disk with the same namespace but a different uuid. It
+            // means this collection must have been dropped and re-created under a different uuid on
+            // the donor during the recipient failover. And the drop and the re-create will be
+            // covered by the oplog application phase.
+            //
+            // 2) We have a [time series] view on disk with the same namespace. It means the view
+            // must have dropped and created a regular collection with the namespace same as the
+            // dropped view during the recipient failover. The drop view and create collection
+            // will be covered by the oplog application phase.
             LOGV2(5767200,
-                  "TenantCollectionCloner found same namespace with different uuid locally on "
-                  "resume, skipping cloning this collection.",
+                  "Tenant collection cloner: Skipping cloning this collection.",
                   "namespace"_attr = getSourceNss(),
                   "migrationId"_attr = getSharedData()->getMigrationId(),
-                  "tenantId"_attr = getTenantId());
+                  "tenantId"_attr = getTenantId(),
+                  "error"_attr = status);
             return kSkipRemainingStages;
         }
         uassertStatusOKWithContext(status, "Tenant collection cloner: create collection");
diff --git a/src/mongo/db/repl/tenant_database_cloner.cpp b/src/mongo/db/repl/tenant_database_cloner.cpp
index 7c7e93062a5..0cde8bfa265 100644
--- a/src/mongo/db/repl/tenant_database_cloner.cpp
+++ b/src/mongo/db/repl/tenant_database_cloner.cpp
@@ -49,6 +49,10 @@ namespace repl {
 // Failpoint which the tenant database cloner to hang after it has successully run listCollections
 // and recorded the results and the operationTime.
 MONGO_FAIL_POINT_DEFINE(tenantDatabaseClonerHangAfterGettingOperationTime);
+// Failpoint to skip comparing the list of collections that are already cloned, instead it will
+// resume the cloning from the beginning of the list, that's provided by
+// TenantDatabaseCloner::listCollectionsStage.
+MONGO_FAIL_POINT_DEFINE(skiplistExistingCollectionsStage);
 
 TenantDatabaseCloner::TenantDatabaseCloner(const std::string& dbName,
                                            TenantMigrationSharedData* sharedData,
@@ -173,6 +177,15 @@ BaseCloner::AfterStageBehavior TenantDatabaseCloner::listCollectionsStage() {
 }
 
 BaseCloner::AfterStageBehavior TenantDatabaseCloner::listExistingCollectionsStage() {
+    if (MONGO_unlikely(skiplistExistingCollectionsStage.shouldFail())) {
+        LOGV2(6312900,
+              "skiplistExistingCollectionsStage failpoint is enabled. "
+              "Tenant DatabaseCloner resumes cloning",
+              "migrationId"_attr = getSharedData()->getMigrationId(),
+              "tenantId"_attr = _tenantId,
+              "resumeFrom"_attr = _collections.front().first);
+        return kContinueNormally;
+    }
     auto opCtx = cc().makeOperationContext();
     DBDirectClient client(opCtx.get());
     tenantMigrationRecipientInfo(opCtx.get()) =