SERVER-59084 Add includeIndexBuildInfo flag to listIndexes

7 files changed, 239 insertions, 30 deletions

diff --git a/jstests/noPassthrough/list_indexes_index_build_info.js b/jstests/noPassthrough/list_indexes_index_build_info.js
new file mode 100644
index 00000000000..3c3afcf7248
--- /dev/null
+++ b/jstests/noPassthrough/list_indexes_index_build_info.js
@@ -0,0 +1,136 @@
+/**
+ * Tests the listIndexes command's includeIndexBuildInfo flag. When the flag is set, all index specs
+ * should be contained within the 'spec' subdocument rather than in the document itself, and for
+ * indexes which are building, the indexBuildInfo document should be returned next to spec, and
+ * should contain a buildUUID.
+ *
+ * @tags: [requires_replication]
+ */
+
+(function() {
+'use strict';
+
+load("jstests/noPassthrough/libs/index_build.js");  // for IndexBuildTest
+
+/**
+ * Given two listIndexes command results, the first without index build info included and the second
+ * with, ensures that ready and in-progress indexes are formatted correctly in each output.
+ */
+function assertListIndexesOutputsMatch(
+    withoutBuildInfo, withBuildInfo, readyIndexNames, buildingIndexNames) {
+    const allIndexNames = readyIndexNames.concat(buildingIndexNames);
+    assert.eq(
+        withoutBuildInfo.map(i => i.name).sort(),
+        allIndexNames.sort(),
+        "Expected indexes do not match returned indexes in withoutBuildInfo: Expected names: " +
+            tojson(allIndexNames) + ", withoutBuildInfo: " + tojson(withoutBuildInfo));
+    assert.eq(withBuildInfo.map(i => i.spec.name).sort(),
+              allIndexNames.sort(),
+              "Expected indexes do not match returned indexes in withBuildInfo: Expected names: " +
+                  tojson(allIndexNames) + ", withBuildInfo: " + tojson(withBuildInfo));
+
+    for (let i = 0; i < withBuildInfo.length; i++) {
+        assert.eq(
+            withoutBuildInfo[i],
+            withBuildInfo[i].spec,
+            "Expected withBuildInfo spec to contain the same information as withoutBuildInfo: withoutBuildInfo result: " +
+                tojson(withoutBuildInfo[i]) +
+                ", withBuildInfo result: " + tojson(withBuildInfo[i]));
+
+        if (readyIndexNames.includes(withBuildInfo[i].spec.name)) {
+            // Index is done, no indexBuildInfo
+            assert(!withBuildInfo[i].hasOwnProperty('indexBuildInfo'),
+                   "Index expected to be done building had indexBuildInfo: " +
+                       tojson(withBuildInfo[i]));
+        } else {
+            // Index building, should have indexBuildInfo.buildUUID
+            assert(withBuildInfo[i].hasOwnProperty('indexBuildInfo'),
+                   "Index expected to be in-progress building did not have indexBuildInfo: " +
+                       tojson(withBuildInfo[i]));
+            assert(
+                withBuildInfo[i].indexBuildInfo.hasOwnProperty('buildUUID'),
+                "Index expected to be in-progress building did not have indexBuildInfo.buildUUID: " +
+                    tojson(withBuildInfo[i]));
+        }
+    }
+}
+
+const rst = ReplSetTest({nodes: 1});
+rst.startSet();
+rst.initiate();
+
+const conn = rst.getPrimary();
+const db = conn.getDB("test");
+const collName = "test_list_indexes_index_build_info";
+const coll = db.getCollection(collName);
+coll.drop();
+db.createCollection(collName);
+
+// Add data to the collection so that we don't hit createIndexOnEmptyCollection. This is important
+// so that we actually hit the failpoint which is set by IndexBuildTest.pauseIndexBuilds.
+coll.insert({a: 600, b: 700});
+
+// Create a new index.
+const doneIndexName = "a_1";
+assert.commandWorked(
+    db.runCommand({createIndexes: collName, indexes: [{key: {a: 1}, name: doneIndexName}]}));
+
+// Ensure that the format of the listIndexes output still changes in the index build complete case.
+let listIndexesDefaultOutput =
+    assert.commandWorked(db.runCommand({listIndexes: collName})).cursor.firstBatch;
+let listIndexesIncludeIndexBuildInfoOutput =
+    assert.commandWorked(db.runCommand({listIndexes: collName, includeIndexBuildInfo: true}))
+        .cursor.firstBatch;
+assertListIndexesOutputsMatch(
+    listIndexesDefaultOutput, listIndexesIncludeIndexBuildInfoOutput, ["_id_", doneIndexName], []);
+
+// Ensure that the includeBuildUUIDs flag cannot be set to true at the same time as the
+// includeBuildIndexInfo flag, but that cases where they are not both set to true are OK.
+assert.commandFailedWithCode(
+    db.runCommand({listIndexes: collName, includeIndexBuildInfo: true, includeBuildUUIDs: true}),
+    ErrorCodes.InvalidOptions);
+const listIndexesIncludeIndexBuildInfoBuildUUIDsFalseOutput =
+    assert
+        .commandWorked(db.runCommand(
+            {listIndexes: collName, includeIndexBuildInfo: true, includeBuildUUIDs: false}))
+        .cursor.firstBatch;
+assert.eq(listIndexesIncludeIndexBuildInfoBuildUUIDsFalseOutput,
+          listIndexesIncludeIndexBuildInfoOutput);
+const listIndexesIncludeBuildUUIDsIndexBuildInfoFalseOutput =
+    assert
+        .commandWorked(db.runCommand(
+            {listIndexes: collName, includeIndexBuildInfo: false, includeBuildUUIDs: true}))
+        .cursor.firstBatch;
+assert.eq(listIndexesIncludeBuildUUIDsIndexBuildInfoFalseOutput, listIndexesDefaultOutput);
+
+// Create a new index, this time intentionally pausing the index build halfway through in order to
+// test the in-progress index case.
+const buildingIndexName = "b_1";
+IndexBuildTest.pauseIndexBuilds(conn);
+const awaitIndexBuild = IndexBuildTest.startIndexBuild(
+    conn, coll.getFullName(), {b: 1}, {name: buildingIndexName}, [], 0);
+IndexBuildTest.waitForIndexBuildToStart(db, collName, buildingIndexName);
+
+// Wait for the new index to appear in listIndexes output.
+assert.soonNoExcept(() => {
+    listIndexesDefaultOutput =
+        assert.commandWorked(db.runCommand({listIndexes: collName})).cursor.firstBatch;
+    assert(listIndexesDefaultOutput.length == 3);
+    return true;
+});
+
+// Ensure that the format of the listIndexes output changes as expected in the in-progress index
+// case.
+listIndexesIncludeIndexBuildInfoOutput =
+    assert.commandWorked(db.runCommand({listIndexes: collName, includeIndexBuildInfo: true}))
+        .cursor.firstBatch;
+assertListIndexesOutputsMatch(listIndexesDefaultOutput,
+                              listIndexesIncludeIndexBuildInfoOutput,
+                              ["_id_", doneIndexName],
+                              [buildingIndexName]);
+
+IndexBuildTest.resumeIndexBuilds(conn);
+IndexBuildTest.waitForIndexBuildToStop(db, collName, buildingIndexName);
+awaitIndexBuild();
+rst.stopSet();
+})();
diff --git a/src/mongo/db/catalog/list_indexes.cpp b/src/mongo/db/catalog/list_indexes.cpp
index 1518dd0c5b4..780cc3c0b12 100644
--- a/src/mongo/db/catalog/list_indexes.cpp
+++ b/src/mongo/db/catalog/list_indexes.cpp
@@ -54,7 +54,7 @@ namespace mongo {
 
 StatusWith<std::list<BSONObj>> listIndexes(OperationContext* opCtx,
                                            const NamespaceStringOrUUID& ns,
-                                           boost::optional<bool> includeBuildUUIDs) {
+                                           ListIndexesInclude additionalInclude) {
     AutoGetCollectionForReadCommandMaybeLockFree collection(opCtx, ns);
     auto nss = collection.getNss();
     if (!collection) {
@@ -63,13 +63,13 @@ StatusWith<std::list<BSONObj>> listIndexes(OperationContext* opCtx,
                                                             << collection.getNss().ns());
     }
     return StatusWith<std::list<BSONObj>>(
-        listIndexesInLock(opCtx, collection.getCollection(), nss, includeBuildUUIDs));
+        listIndexesInLock(opCtx, collection.getCollection(), nss, additionalInclude));
 }
 
 std::list<BSONObj> listIndexesInLock(OperationContext* opCtx,
                                      const CollectionPtr& collection,
                                      const NamespaceString& nss,
-                                     boost::optional<bool> includeBuildUUIDs) {
+                                     ListIndexesInclude additionalInclude) {
     CurOpFailpointHelpers::waitWhileFailPointEnabled(
         &hangBeforeListIndexes, opCtx, "hangBeforeListIndexes", []() {}, nss);
 
@@ -79,34 +79,53 @@ std::list<BSONObj> listIndexesInLock(OperationContext* opCtx,
         collection->getAllIndexes(&indexNames);
 
         if (collection->isClustered() && !collection->ns().isTimeseriesBucketsCollection()) {
-            indexSpecs.push_back(clustered_util::formatClusterKeyForListIndexes(
-                collection->getClusteredInfo().get()));
+            auto clusteredSpec = clustered_util::formatClusterKeyForListIndexes(
+                collection->getClusteredInfo().get());
+            if (additionalInclude == ListIndexesInclude::IndexBuildInfo) {
+                indexSpecs.push_back(BSON("spec"_sd << clusteredSpec));
+            } else {
+                indexSpecs.push_back(clusteredSpec);
+            }
         }
         for (size_t i = 0; i < indexNames.size(); i++) {
-            if (!includeBuildUUIDs.value_or(false) || collection->isIndexReady(indexNames[i])) {
-                indexSpecs.push_back(collection->getIndexSpec(indexNames[i]));
-                continue;
-            }
+            auto spec = collection->getIndexSpec(indexNames[i]);
+            auto durableBuildUUID = collection->getIndexBuildUUID(indexNames[i]);
             // The durable catalog will not have a build UUID for the given index name if it was
-            // not being built with two-phase.
-            const auto durableBuildUUID = collection->getIndexBuildUUID(indexNames[i]);
-            if (!durableBuildUUID) {
-                indexSpecs.push_back(collection->getIndexSpec(indexNames[i]));
-                continue;
+            // not being built with two-phase -- in this case we have no relevant index build info
+            bool inProgressInformationExists =
+                !collection->isIndexReady(indexNames[i]) && durableBuildUUID;
+            switch (additionalInclude) {
+                case ListIndexesInclude::Nothing:
+                    indexSpecs.push_back(spec);
+                    break;
+                case ListIndexesInclude::BuildUUID:
+                    if (inProgressInformationExists) {
+                        indexSpecs.push_back(
+                            BSON("spec"_sd << spec << "buildUUID"_sd << *durableBuildUUID));
+                    } else {
+                        indexSpecs.push_back(spec);
+                    }
+                    break;
+                case ListIndexesInclude::IndexBuildInfo:
+                    if (inProgressInformationExists) {
+                        indexSpecs.push_back(BSON("spec"_sd
+                                                  << spec << "indexBuildInfo"_sd
+                                                  << BSON("buildUUID"_sd << *durableBuildUUID)));
+                    } else {
+                        indexSpecs.push_back(BSON("spec"_sd << spec));
+                    }
+                    break;
+                default:
+                    MONGO_UNREACHABLE;
             }
-
-            BSONObjBuilder builder;
-            builder.append("spec"_sd, collection->getIndexSpec(indexNames[i]));
-            durableBuildUUID->appendToBuilder(&builder, "buildUUID"_sd);
-            indexSpecs.push_back(builder.obj());
         }
         return indexSpecs;
     });
 }
 std::list<BSONObj> listIndexesEmptyListIfMissing(OperationContext* opCtx,
                                                  const NamespaceStringOrUUID& nss,
-                                                 boost::optional<bool> includeBuildUUIDs) {
-    auto listStatus = listIndexes(opCtx, nss, includeBuildUUIDs);
+                                                 ListIndexesInclude additionalInclude) {
+    auto listStatus = listIndexes(opCtx, nss, additionalInclude);
     return listStatus.isOK() ? listStatus.getValue() : std::list<BSONObj>();
 }
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/list_indexes.h b/src/mongo/db/catalog/list_indexes.h
index fd3053fa75f..03df1c200c9 100644
--- a/src/mongo/db/catalog/list_indexes.h
+++ b/src/mongo/db/catalog/list_indexes.h
@@ -38,17 +38,23 @@
 namespace mongo {
 
 /**
+ * Corresponds to flags passed to listIndexes which specify additional information to be returned
+ * for each index. BuildUUID = includeBuildUUIDs flag IndexBuildInfo = includeIndexBuildInfo flag
+ */
+enum ListIndexesInclude { Nothing, BuildUUID, IndexBuildInfo };
+
+/**
  * Return a list of the indexes on the given collection.
  */
 StatusWith<std::list<BSONObj>> listIndexes(OperationContext* opCtx,
                                            const NamespaceStringOrUUID& ns,
-                                           boost::optional<bool> includeBuildUUIDs);
+                                           ListIndexesInclude additionalInclude);
 std::list<BSONObj> listIndexesInLock(OperationContext* opCtx,
                                      const CollectionPtr& collection,
                                      const NamespaceString& nss,
-                                     boost::optional<bool> includeBuildUUIDs);
+                                     ListIndexesInclude additionalInclude);
 std::list<BSONObj> listIndexesEmptyListIfMissing(OperationContext* opCtx,
                                                  const NamespaceStringOrUUID& nss,
-                                                 boost::optional<bool> includeBuildUUIDs);
+                                                 ListIndexesInclude additionalInclude);
 
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp
index d4b909f0cff..e80d1a9aaa6 100644
--- a/src/mongo/db/catalog/rename_collection.cpp
+++ b/src/mongo/db/catalog/rename_collection.cpp
@@ -765,7 +765,7 @@ void doLocalRenameIfOptionsAndIndexesHaveNotChanged(OperationContext* opCtx,
                 originalCollectionOptions.removeField("uuid") == collectionOptions));
 
     auto currentIndexes =
-        listIndexesEmptyListIfMissing(opCtx, targetNs, false /* includeBuildUUIDs */);
+        listIndexesEmptyListIfMissing(opCtx, targetNs, ListIndexesInclude::Nothing);
 
     UnorderedFieldsBSONObjComparator comparator;
     uassert(
diff --git a/src/mongo/db/commands/list_indexes.cpp b/src/mongo/db/commands/list_indexes.cpp
index 95504958339..89fde689852 100644
--- a/src/mongo/db/commands/list_indexes.cpp
+++ b/src/mongo/db/commands/list_indexes.cpp
@@ -71,6 +71,13 @@ IndexSpecsWithNamespaceString getIndexSpecsWithNamespaceString(OperationContext*
                                                                const ListIndexes& cmd) {
     const auto& origNssOrUUID = cmd.getNamespaceOrUUID();
 
+    bool buildUUID = cmd.getIncludeBuildUUIDs().value_or(false);
+    bool indexBuildInfo = cmd.getIncludeIndexBuildInfo().value_or(false);
+    invariant(!(buildUUID && indexBuildInfo));
+    ListIndexesInclude additionalInclude = buildUUID
+        ? ListIndexesInclude::BuildUUID
+        : indexBuildInfo ? ListIndexesInclude::IndexBuildInfo : ListIndexesInclude::Nothing;
+
     // Since time-series collections don't have UUIDs, we skip the time-series lookup
     // if the target collection is specified as a UUID.
     if (const auto& origNss = origNssOrUUID.nss()) {
@@ -91,7 +98,7 @@ IndexSpecsWithNamespaceString getIndexSpecsWithNamespaceString(OperationContext*
             return std::make_pair(
                 timeseries::createTimeseriesIndexesFromBucketsIndexes(
                     *timeseriesOptions,
-                    listIndexesInLock(opCtx, coll, bucketsNss, cmd.getIncludeBuildUUIDs())),
+                    listIndexesInLock(opCtx, coll, bucketsNss, additionalInclude)),
                 bucketsNss.getTimeseriesViewNamespace());
         }
     }
@@ -103,18 +110,22 @@ IndexSpecsWithNamespaceString getIndexSpecsWithNamespaceString(OperationContext*
     uassert(
         ErrorCodes::NamespaceNotFound, str::stream() << "ns does not exist: " << nss.ns(), coll);
 
-    return std::make_pair(listIndexesInLock(opCtx, coll, nss, cmd.getIncludeBuildUUIDs()), nss);
+    return std::make_pair(listIndexesInLock(opCtx, coll, nss, additionalInclude), nss);
 }
 
 /**
  * Lists the indexes for a given collection.
  * If 'includeBuildUUIDs' is true, then the index build uuid is also returned alongside the index
  * spec for in-progress index builds only.
+ * If 'includeIndexBuildInfo' is true, then the index spec is returned in the spec subdocument, and
+ * index build info is returned alongside the index spec for in-progress index builds only.
+ * includeBuildUUIDs and includeIndexBuildInfo cannot both be set to true.
  *
  * Format:
  * {
  *   listIndexes: <collection name>,
  *   includeBuildUUIDs: <boolean>,
+ *   includeIndexBuildInfo: <boolean>
  * }
  *
  * Return format:
@@ -132,6 +143,19 @@ IndexSpecsWithNamespaceString getIndexSpecsWithNamespaceString(OperationContext*
  *   spec: <index spec>,
  *   buildUUID: <index build uuid>
  * }
+ *
+ * If 'includeIndexBuildInfo' is true, then for in-progress indexes, <index> has the following
+ * format:
+ * {
+ *   spec: <index spec>,
+ *   indexBuildInfo: {
+ *     buildUUID: <index build uuid>
+ *   }
+ * }
+ * And for complete (not in-progress) indexes:
+ * {
+ *   spec: <index spec>
+ * }
  */
 
 class CmdListIndexes final : public ListIndexesCmdVersion1Gen<CmdListIndexes> {
@@ -192,7 +216,14 @@ public:
 
         ListIndexesReply typedRun(OperationContext* opCtx) final {
             CommandHelpers::handleMarkKillOnClientDisconnect(opCtx);
-            auto indexSpecsWithNss = getIndexSpecsWithNamespaceString(opCtx, request());
+            const auto& cmd = request();
+            bool buildUUID = cmd.getIncludeBuildUUIDs().value_or(false);
+            bool indexBuildInfo = cmd.getIncludeIndexBuildInfo().value_or(false);
+            uassert(ErrorCodes::InvalidOptions,
+                    "The includeBuildUUIDs flag and includeBuildIndexInfo flag cannot both be set "
+                    "to true",
+                    !(buildUUID && indexBuildInfo));
+            auto indexSpecsWithNss = getIndexSpecsWithNamespaceString(opCtx, cmd);
             const auto& indexList = indexSpecsWithNss.first;
             const auto& nss = indexSpecsWithNss.second;
             return ListIndexesReply(_makeCursor(opCtx, indexList, nss));
diff --git a/src/mongo/db/list_indexes.idl b/src/mongo/db/list_indexes.idl
index dde5cea8ee8..9348ce7d102 100644
--- a/src/mongo/db/list_indexes.idl
+++ b/src/mongo/db/list_indexes.idl
@@ -157,14 +157,25 @@ structs:
                 optional: true
                 unstable: false
             #
-            # An index build in progress appears with these two fields. They're required, but marked
+            # Depending on the values of includeIndexBuildInfo and includeBuildUUIDs, indexes may
+            # appear with a combination of these three fields. Specifically, if includeIndexBuildInfo 
+            # is set, completed index builds will appear with spec, and in-progress index builds will
+            # appear with both spec and indexBuildInfo, while if includeBuildUUIDs is set, in-progress
+            # index builds will appear with both spec and buildUUID. They're required, but marked
             # optional to permit the built index format using the fields above instead.
             #
+            indexBuildInfo:
+                description: "Contains information about the build. Included if includeIndexBuildInfo is set and the index is currently building."
+                type: object_owned
+                optional: true
+                unstable: true
             buildUUID:
+                description: "The UUID of a two-phase index build. Included if includeBuildUUIDs is set and the index is currently building."
                 type: uuid
                 optional: true
                 unstable: false
             spec:
+                description: "Contains the index spec. If includeIndexBuildInfo is set, always included. If includeBuildUUIDs is set, included if the index is currently building."
                 type: NewIndexSpec
                 optional: true
                 unstable: false
@@ -221,6 +232,11 @@ commands:
                 type: safeBool
                 optional: true
                 unstable: true
+            includeIndexBuildInfo:
+                description: "If true, the reply will include information about the build status for each index."
+                type: safeBool
+                optional: true
+                unstable: true
             isTimeseriesNamespace:
                 description: "This flag is set to true when the command was originally sent to
                               mongos on the time-series view, but got rewritten to target
diff --git a/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp b/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
index fb1e4b1e976..cb29fdd6b51 100644
--- a/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
+++ b/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
@@ -55,7 +55,8 @@ NonShardServerProcessInterface::attachCursorSourceToPipeline(
 std::list<BSONObj> NonShardServerProcessInterface::getIndexSpecs(OperationContext* opCtx,
                                                                  const NamespaceString& ns,
                                                                  bool includeBuildUUIDs) {
-    return listIndexesEmptyListIfMissing(opCtx, ns, includeBuildUUIDs);
+    return listIndexesEmptyListIfMissing(
+        opCtx, ns, includeBuildUUIDs ? ListIndexesInclude::BuildUUID : ListIndexesInclude::Nothing);
 }
 
 std::pair<std::vector<FieldPath>, bool>