SERVER-36968 Rebuild interrupted system indexes before checking AuthZN index presence

(cherry picked from commit 3f7235ad61a16702e2a5200a912e3e02dd89a44e)

5 files changed, 120 insertions, 30 deletions

diff --git a/jstests/auth/rebuild_system_indexes.js b/jstests/auth/rebuild_system_indexes.js
new file mode 100644
index 00000000000..65d43867a79
--- /dev/null
+++ b/jstests/auth/rebuild_system_indexes.js
@@ -0,0 +1,53 @@
+// This test verifies that the server will attempt to rebuild admin.system.users index on startup if
+// it is in a partially built state.
+
+(() => {
+    "use strict";
+
+    const dbpath = "rebuild_system_indexes";
+    const user = 'admin';
+    const pwd = 'adminPassword';
+
+    const mongod = MongoRunner.runMongod({storageEngine: 'mmapv1', dbpath});
+
+    let admin = mongod.getDB('admin');
+
+    // Create the system.users index
+    assert.commandWorked(
+        admin.runCommand({createUser: user, pwd, roles: [{role: 'root', db: 'admin'}]}),
+        'failed to create user/indexes');
+    assert.commandWorked(admin.runCommand({deleteIndexes: 'system.users', index: '*'}),
+                         'failed to drop indexes');
+
+    MongoRunner.stopMongod(mongod);
+
+    // Add a fail point so the server crashes while rebuilding the indexes
+    print('================= starting mongod that WILL CRASH PURPOSEFULLY =================');
+    MongoRunner.runMongod({
+        restart: mongod,
+        storageEngine: 'mmapv1',
+        setParameter: "failpoint.crashAfterStartingIndexBuild={mode: 'alwaysOn'}", dbpath,
+    });
+    print('======================== END PURPOSEFUL CRASH ========================');
+
+    // Make sure the server isnt running
+    assert.neq(0,
+               runMongoProgram('mongo', '--port', mongod.port, '--eval', 'db.isMaster()'),
+               "mongod did not crash when creating index");
+
+    // Start up normally, indexes should be rebuilt
+    MongoRunner.runMongod({
+        restart: mongod,
+        storageEngine: 'mmapv1',
+        setParameter: "failpoint.crashAfterStartingIndexBuild={mode: 'off'}", dbpath,
+    });
+
+    assert(mongod);
+
+    admin = mongod.getDB('admin');
+    admin.auth(user, pwd);
+
+    assert(admin.getCollection('system.users').getIndexes().length > 1, 'indexes were not rebuilt');
+
+    MongoRunner.stopMongod(mongod);
+})();
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript
index d184bcd91ee..9ab05522651 100644
--- a/src/mongo/db/SConscript
+++ b/src/mongo/db/SConscript
@@ -586,6 +586,9 @@ env.Library(
         'db_raii',
         'catalog/index_key_validate',
     ],
+    LIBDEPS_PRIVATE=[
+        'index_d',
+    ]
 )
 
 env.Library(
diff --git a/src/mongo/db/index_rebuilder.cpp b/src/mongo/db/index_rebuilder.cpp
index 1367792ff3f..1a8bbf59d6c 100644
--- a/src/mongo/db/index_rebuilder.cpp
+++ b/src/mongo/db/index_rebuilder.cpp
@@ -52,12 +52,10 @@
 
 namespace mongo {
 
-using std::endl;
-using std::string;
-using std::vector;
-
 namespace {
-void checkNS(OperationContext* opCtx, const std::list<std::string>& nsToCheck) {
+void checkNS(OperationContext* opCtx,
+             const std::list<std::string>& nsToCheck,
+             bool overrideNoIndexBuildRetry) {
     bool firstTime = true;
     for (std::list<std::string>::const_iterator it = nsToCheck.begin(); it != nsToCheck.end();
          ++it) {
@@ -86,7 +84,7 @@ void checkNS(OperationContext* opCtx, const std::list<std::string>& nsToCheck) {
 
         {
             WriteUnitOfWork wunit(opCtx);
-            vector<BSONObj> indexesToBuild = indexCatalog->getAndClearUnfinishedIndexes(opCtx);
+            std::vector<BSONObj> indexesToBuild = indexCatalog->getAndClearUnfinishedIndexes(opCtx);
 
             // The indexes have now been removed from system.indexes, so the only record is
             // in-memory. If there is a journal commit between now and when insert() rewrites
@@ -104,12 +102,15 @@ void checkNS(OperationContext* opCtx, const std::list<std::string>& nsToCheck) {
                   << nss.ns();
 
             if (firstTime) {
-                log() << "note: restart the server with --noIndexBuildRetry "
-                      << "to skip index rebuilds";
                 firstTime = false;
+
+                if (!overrideNoIndexBuildRetry) {
+                    log() << "note: restart the server with --noIndexBuildRetry "
+                          << "to skip index rebuilds";
+                }
             }
 
-            if (!serverGlobalParams.indexBuildRetry) {
+            if (!serverGlobalParams.indexBuildRetry && !overrideNoIndexBuildRetry) {
                 log() << "  not rebuilding interrupted indexes";
                 wunit.commit();
                 continue;
@@ -143,6 +144,11 @@ void checkNS(OperationContext* opCtx, const std::list<std::string>& nsToCheck) {
 }
 }  // namespace
 
+void forceRestartInProgressIndexesOnCollection(OperationContext* opCtx, const NamespaceString& ns) {
+    std::list<std::string> namespaces = {ns.ns()};
+    checkNS(opCtx, namespaces, true);
+}
+
 void restartInProgressIndexesFromLastShutdown(OperationContext* opCtx) {
     AuthorizationSession::get(opCtx->getClient())->grantInternalAuthorization();
 
@@ -161,7 +167,7 @@ void restartInProgressIndexesFromLastShutdown(OperationContext* opCtx) {
             Database* db = autoDb.getDb();
             db->getDatabaseCatalogEntry()->getCollectionNamespaces(&collNames);
         }
-        checkNS(opCtx, collNames);
+        checkNS(opCtx, collNames, false);
     } catch (const DBException& e) {
         error() << "Index verification did not complete: " << redact(e);
         fassertFailedNoTrace(18643);
diff --git a/src/mongo/db/index_rebuilder.h b/src/mongo/db/index_rebuilder.h
index 2e08a972c66..34130173340 100644
--- a/src/mongo/db/index_rebuilder.h
+++ b/src/mongo/db/index_rebuilder.h
@@ -30,6 +30,8 @@
 
 #pragma once
 
+#include "mongo/db/namespace_string.h"
+
 namespace mongo {
 
 class OperationContext;
@@ -39,4 +41,11 @@ class OperationContext;
  * Only call this at startup before taking requests.
  */
 void restartInProgressIndexesFromLastShutdown(OperationContext* opCtx);
+
+/**
+ * Restarts building indexes that were in progress during shutdown on a single collection.
+ * Only call this at startup before taking requests.
+ * NOTE: This function ignores the --noIndexBuildRetry option
+ */
+void forceRestartInProgressIndexesOnCollection(OperationContext* opCtx, const NamespaceString& ns);
 }
diff --git a/src/mongo/db/system_index.cpp b/src/mongo/db/system_index.cpp
index 4fde4b1b570..1f7e26f74e9 100644
--- a/src/mongo/db/system_index.cpp
+++ b/src/mongo/db/system_index.cpp
@@ -48,6 +48,7 @@
 #include "mongo/db/curop.h"
 #include "mongo/db/db_raii.h"
 #include "mongo/db/index/index_descriptor.h"
+#include "mongo/db/index_rebuilder.h"
 #include "mongo/db/jsobj.h"
 #include "mongo/db/storage/storage_options.h"
 #include "mongo/util/assert_util.h"
@@ -149,6 +150,36 @@ void generateSystemIndexForExistingCollection(OperationContext* opCtx,
     }
 }
 
+Status createOrRebuildIndex(OperationContext* opCtx,
+                            Collection* collection,
+                            const BSONObj& indexPattern,
+                            const IndexSpec& indexSpec) {
+    IndexCatalog* indexCatalog = collection->getIndexCatalog();
+    invariant(indexCatalog);
+
+    if (!indexCatalog->checkUnfinished().isOK()) {
+        try {
+            forceRestartInProgressIndexesOnCollection(opCtx, collection->ns());
+        } catch (...) {
+            return exceptionToStatus();
+        }
+    }
+
+    std::vector<IndexDescriptor*> indexes;
+    indexCatalog->findIndexesByKeyPattern(opCtx, indexPattern, false, &indexes);
+
+    if (indexes.empty()) {
+        try {
+            generateSystemIndexForExistingCollection(
+                opCtx, collection, collection->ns(), indexSpec);
+        } catch (...) {
+            return exceptionToStatus();
+        }
+    }
+
+    return Status::OK();
+}
+
 }  // namespace
 
 Status verifySystemIndexes(OperationContext* opCtx) {
@@ -180,32 +211,20 @@ Status verifySystemIndexes(OperationContext* opCtx) {
             }
 
             // Ensure that system indexes exist for the user collection
-            indexCatalog->findIndexesByKeyPattern(opCtx, v3SystemUsersKeyPattern, false, &indexes);
-            if (indexes.empty()) {
-                try {
-                    generateSystemIndexForExistingCollection(
-                        opCtx, collection, systemUsers, v3SystemUsersIndexSpec);
-                } catch (...) {
-                    return exceptionToStatus();
-                }
+            Status createRebuildStatus = createOrRebuildIndex(
+                opCtx, collection, v3SystemUsersKeyPattern, v3SystemUsersIndexSpec);
+            if (!createRebuildStatus.isOK()) {
+                return createRebuildStatus;
             }
         }
 
         // Ensure that system indexes exist for the roles collection, if it exists.
         collection = autoDb.getDb()->getCollection(opCtx, systemRoles);
         if (collection) {
-            IndexCatalog* indexCatalog = collection->getIndexCatalog();
-            invariant(indexCatalog);
-
-            std::vector<IndexDescriptor*> indexes;
-            indexCatalog->findIndexesByKeyPattern(opCtx, v3SystemRolesKeyPattern, false, &indexes);
-            if (indexes.empty()) {
-                try {
-                    generateSystemIndexForExistingCollection(
-                        opCtx, collection, systemRoles, v3SystemRolesIndexSpec);
-                } catch (...) {
-                    return exceptionToStatus();
-                }
+            Status createRebuildStatus = createOrRebuildIndex(
+                opCtx, collection, v3SystemRolesKeyPattern, v3SystemRolesIndexSpec);
+            if (!createRebuildStatus.isOK()) {
+                return createRebuildStatus;
             }
         }
     }