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

5 files changed, 126 insertions, 32 deletions

diff --git a/jstests/auth/rebuild_system_indexes.js b/jstests/auth/rebuild_system_indexes.js
new file mode 100644
index 00000000000..76ccd52c5a1
--- /dev/null
+++ b/jstests/auth/rebuild_system_indexes.js
@@ -0,0 +1,56 @@
+/**
+ * This test verifies that the server will attempt to rebuild admin.system.users index on startup if
+ * it is in a partially built state using the mmapv1 storage engine.
+ *
+ * @tags: [requires_journaling, requires_mmapv1]
+ */
+(() => {
+    "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/auth/SConscript b/src/mongo/db/auth/SConscript
index fc85149de7f..9ef91280636 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -145,6 +145,10 @@ env.Library('authmongod',
                 '$BUILD_DIR/mongo/db/repl/repl_coordinator_global',
                 '$BUILD_DIR/mongo/db/server_parameters',
             ],
+            LIBDEPS_TAGS=[
+                # Depends on index rebuilder from serveronly
+                'incomplete'
+            ]
 )
 
 env.Library('authmongos',
diff --git a/src/mongo/db/auth/auth_index_d.cpp b/src/mongo/db/auth/auth_index_d.cpp
index 9944a582f5c..0918dfa798e 100644
--- a/src/mongo/db/auth/auth_index_d.cpp
+++ b/src/mongo/db/auth/auth_index_d.cpp
@@ -46,6 +46,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/repl/replication_coordinator.h"
 #include "mongo/db/storage/storage_options.h"
@@ -152,6 +153,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* txn) {
@@ -183,36 +214,23 @@ Status verifySystemIndexes(OperationContext* txn) {
         }
 
         // Ensure that system indexes exist for the user collection
-        indexCatalog->findIndexesByKeyPattern(txn, v3SystemUsersKeyPattern, false, &indexes);
-        if (indexes.empty()) {
-            try {
-                generateSystemIndexForExistingCollection(
-                    txn, collection, systemUsers, v3SystemUsersIndexSpec);
-            } catch (...) {
-                return exceptionToStatus();
-            }
+        Status createRebuildStatus =
+            createOrRebuildIndex(txn, collection, v3SystemUsersKeyPattern, v3SystemUsersIndexSpec);
+        if (!createRebuildStatus.isOK()) {
+            return createRebuildStatus;
         }
     }
 
     // Ensure that system indexes exist for the roles collection, if it exists.
     collection = autoDb.getDb()->getCollection(systemRoles);
     if (collection) {
-        IndexCatalog* indexCatalog = collection->getIndexCatalog();
-        invariant(indexCatalog);
-
-        std::vector<IndexDescriptor*> indexes;
-        indexCatalog->findIndexesByKeyPattern(txn, v3SystemRolesKeyPattern, false, &indexes);
-        if (indexes.empty()) {
-            try {
-                generateSystemIndexForExistingCollection(
-                    txn, collection, systemRoles, v3SystemRolesIndexSpec);
-            } catch (...) {
-                return exceptionToStatus();
-            }
+        Status createRebuildStatus =
+            createOrRebuildIndex(txn, collection, v3SystemRolesKeyPattern, v3SystemRolesIndexSpec);
+        if (!createRebuildStatus.isOK()) {
+            return createRebuildStatus;
         }
     }
 
-
     return Status::OK();
 }
 
diff --git a/src/mongo/db/index_rebuilder.cpp b/src/mongo/db/index_rebuilder.cpp
index 2d4c40f5747..de00ef920d1 100644
--- a/src/mongo/db/index_rebuilder.cpp
+++ b/src/mongo/db/index_rebuilder.cpp
@@ -51,16 +51,14 @@
 
 namespace mongo {
 
-using std::endl;
-using std::string;
-using std::vector;
-
 namespace {
-void checkNS(OperationContext* txn, const std::list<std::string>& nsToCheck) {
+void checkNS(OperationContext* txn,
+             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) {
-        string ns = *it;
+        std::string ns = *it;
 
         LOG(3) << "IndexRebuilder::checkNS: " << ns;
 
@@ -87,7 +85,7 @@ void checkNS(OperationContext* txn, const std::list<std::string>& nsToCheck) {
 
         {
             WriteUnitOfWork wunit(txn);
-            vector<BSONObj> indexesToBuild = indexCatalog->getAndClearUnfinishedIndexes(txn);
+            std::vector<BSONObj> indexesToBuild = indexCatalog->getAndClearUnfinishedIndexes(txn);
 
             // 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* txn, const std::list<std::string>& nsToCheck) {
             log() << "found " << indexesToBuild.size() << " interrupted index build(s) on " << 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* txn, 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* txn) {
     AuthorizationSession::get(txn->getClient())->grantInternalAuthorization();
 
@@ -162,7 +168,8 @@ void restartInProgressIndexesFromLastShutdown(OperationContext* txn) {
             Database* db = autoDb.getDb();
             db->getDatabaseCatalogEntry()->getCollectionNamespaces(&collNames);
         }
-        checkNS(txn, collNames);
+
+        checkNS(txn, 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 bf01367e783..49ac4e43db9 100644
--- a/src/mongo/db/index_rebuilder.h
+++ b/src/mongo/db/index_rebuilder.h
@@ -28,6 +28,8 @@
 
 #pragma once
 
+#include "mongo/db/namespace_string.h"
+
 namespace mongo {
 
 class OperationContext;
@@ -37,4 +39,11 @@ class OperationContext;
  * Only call this at startup before taking requests.
  */
 void restartInProgressIndexesFromLastShutdown(OperationContext* txn);
+
+/**
+ * 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);
 }