diff options
-rw-r--r-- | jstests/noPassthrough/list_indexes_index_build_info.js | 136 | ||||
-rw-r--r-- | src/mongo/db/catalog/list_indexes.cpp | 61 | ||||
-rw-r--r-- | src/mongo/db/catalog/list_indexes.h | 12 | ||||
-rw-r--r-- | src/mongo/db/catalog/rename_collection.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands/list_indexes.cpp | 37 | ||||
-rw-r--r-- | src/mongo/db/list_indexes.idl | 18 | ||||
-rw-r--r-- | src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp | 3 |
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> |