diff options
author | Pol PiƱol Castuera <67922619+PolPinol@users.noreply.github.com> | 2022-11-17 07:54:58 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-11-17 08:32:23 +0000 |
commit | 841b110f721d836fc97ca9d26424038a8268564d (patch) | |
tree | 8b9d149c8d1b4b009e84c6c02b364e7d17a8f1e9 | |
parent | 72063954fe932e99fcd94bb5b30fe18345a14291 (diff) | |
download | mongo-841b110f721d836fc97ca9d26424038a8268564d.tar.gz |
SERVER-68769 If the shard key index cannot be dropped, it cannot be hidden.
-rw-r--r-- | jstests/libs/index_catalog_helpers.js | 85 | ||||
-rw-r--r-- | jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js | 7 | ||||
-rw-r--r-- | jstests/sharding/hidden_index.js | 80 | ||||
-rw-r--r-- | jstests/sharding/timeseries_coll_mod.js | 9 | ||||
-rw-r--r-- | src/mongo/db/catalog/coll_mod.cpp | 22 | ||||
-rw-r--r-- | src/mongo/db/catalog/drop_indexes.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/s/shard_key_index_util.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/s/shard_key_index_util.h | 15 | ||||
-rw-r--r-- | src/mongo/db/s/shard_key_index_util_test.cpp | 11 |
9 files changed, 225 insertions, 30 deletions
diff --git a/jstests/libs/index_catalog_helpers.js b/jstests/libs/index_catalog_helpers.js new file mode 100644 index 00000000000..a61267fbbbd --- /dev/null +++ b/jstests/libs/index_catalog_helpers.js @@ -0,0 +1,85 @@ +"use strict"; + +/** + * Helper functions that help test things to do with the index catalog. + */ +var IndexCatalogHelpers = (function() { + /** + * Returns the index specification with the name 'indexName' if it is present in the + * 'indexSpecs' array, and returns null otherwise. + */ + function getIndexSpecByName(indexSpecs, indexName) { + if (typeof indexName !== "string") { + throw new Error("'indexName' parameter must be a string, but got " + tojson(indexName)); + } + + const found = indexSpecs.filter(spec => spec.name === indexName); + + if (found.length > 1) { + throw new Error("Found multiple indexes with name '" + indexName + + "': " + tojson(indexSpecs)); + } + return (found.length === 1) ? found[0] : null; + } + + /** + * Returns the index specification with the key pattern 'keyPattern' and the collation + * 'collation' if it is present in the 'indexSpecs' array, and returns null otherwise. + * + * The 'collation' parameter is optional and is only required to be specified when multiple + * indexes with the same key pattern exist. + */ + function getIndexSpecByKeyPattern(indexSpecs, keyPattern, collation) { + const collationWasSpecified = arguments.length >= 3; + const foundByKeyPattern = indexSpecs.filter(spec => { + return bsonWoCompare(spec.key, keyPattern) === 0; + }); + + if (!collationWasSpecified) { + if (foundByKeyPattern.length > 1) { + throw new Error( + "Found multiple indexes with key pattern " + tojson(keyPattern) + + " and 'collation' parameter was not specified: " + tojson(indexSpecs)); + } + return (foundByKeyPattern.length === 1) ? foundByKeyPattern[0] : null; + } + + const foundByKeyPatternAndCollation = foundByKeyPattern.filter(spec => { + if (collation.locale === "simple") { + // The simple collation is not explicitly stored in the index catalog, so we expect + // the "collation" field to be absent. + return !spec.hasOwnProperty("collation"); + } + return bsonWoCompare(spec.collation, collation) === 0; + }); + + if (foundByKeyPatternAndCollation.length > 1) { + throw new Error("Found multiple indexes with key pattern" + tojson(keyPattern) + + " and collation " + tojson(collation) + ": " + tojson(indexSpecs)); + } + return (foundByKeyPatternAndCollation.length === 1) ? foundByKeyPatternAndCollation[0] + : null; + } + + function createSingleIndex(coll, key, parameters) { + return coll.getDB().runCommand( + {createIndexes: coll.getName(), indexes: [Object.assign({key: key}, parameters)]}); + } + + function createIndexAndVerifyWithDrop(coll, key, parameters) { + coll.dropIndexes(); + assert.commandWorked(createSingleIndex(coll, key, parameters)); + assert.neq( + null, + getIndexSpecByName(coll.getIndexes(), parameters.name), + () => + `Could not find index with name ${parameters.name}: ${tojson(coll.getIndexes())}`); + } + + return { + findByName: getIndexSpecByName, + findByKeyPattern: getIndexSpecByKeyPattern, + createSingleIndex: createSingleIndex, + createIndexAndVerifyWithDrop: createIndexAndVerifyWithDrop, + }; +})(); diff --git a/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js b/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js index 12c0f4bf6ef..ae9b843e445 100644 --- a/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js +++ b/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js @@ -32,11 +32,12 @@ assert.commandWorked(mongos.adminCommand({ assert.commandWorked(mongos.adminCommand({setFeatureCompatibilityVersion: '5.0'})); const oldDb = st.s1.getDB(dbName); + +assert.commandWorked(db[collName].createIndex({[timeField]: 1})); // Assert that collMod works with matching versions of mongos and mongod. -assert.commandWorked(db.runCommand({collMod: collName, index: {name: indexName, hidden: true}})); +assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'tm_1', hidden: true}})); // Assert that collMod still works with old version of mongos. -assert.commandWorked( - oldDb.runCommand({collMod: collName, index: {name: indexName, hidden: false}})); +assert.commandWorked(oldDb.runCommand({collMod: collName, index: {name: 'tm_1', hidden: false}})); // Assert that collMod with granularity update fails with matching versions of mongos and mongod. assert.commandFailedWithCode(db.runCommand({collMod: collName, timeseries: {granularity: 'hours'}}), diff --git a/jstests/sharding/hidden_index.js b/jstests/sharding/hidden_index.js new file mode 100644 index 00000000000..6bf2c00cdf5 --- /dev/null +++ b/jstests/sharding/hidden_index.js @@ -0,0 +1,80 @@ +/** + * Test to validate that a shard key index cannot be hidden if it cannot be dropped. + * @tags: [ + * requires_fcv_60, + * ] + */ + +(function() { +'use strict'; +load("jstests/libs/index_catalog_helpers.js"); // For IndexCatalogHelpers.findByName. + +// Test to validate the correct behaviour of hiding an index in a sharded cluster with a shard key. +function validateHiddenIndexBehaviour() { + let index_type = 1; + let index_name = "a_" + index_type; + assert.commandWorked(coll.createIndex({"a": index_type})); + + let idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name); + assert.eq(idxSpec.hidden, undefined); + + assert.commandWorked(coll.hideIndex(index_name)); + idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name); + assert(idxSpec.hidden); + + assert.commandWorked(coll.unhideIndex(index_name)); + idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name); + assert.eq(idxSpec.hidden, undefined); + + assert.commandWorked(coll.dropIndex(index_name)); + assert.commandWorked(coll.createIndex({"a": index_type}, {hidden: true})); + + idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name); + assert(idxSpec.hidden); + assert.commandWorked(coll.dropIndexes()); +} + +// Check that command will fail when we try to hide the only shard key index of the collection +function validateOneShardKeyHiddenIndexBehaviour() { + assert.commandFailedWithCode(coll.hideIndex("skey_1"), ErrorCodes.InvalidOptions); + assert.commandFailedWithCode( + testDb.runCommand({"collMod": coll.getName(), "index": {"name": "skey_1", "hidden": true}}), + ErrorCodes.InvalidOptions); +} + +// Check that command will fail when we try to hide or drop an index that is the last shard key +// index of the collection +function validateDifferentHiddenIndexesBehaviour() { + // Create index on skey + assert.commandWorked(coll.createIndex({skey: 1, anotherkey: 1})); + + assert.commandWorked(testDb.runCommand( + {"collMod": coll.getName(), "index": {"name": "skey_1", "hidden": true}})); + + assert.commandFailedWithCode( + testDb.runCommand( + {"collMod": coll.getName(), "index": {"name": "skey_1_anotherkey_1", "hidden": true}}), + ErrorCodes.InvalidOptions); + + assert.commandFailed(coll.dropIndex("skey_1_anotherkey_1")); +} + +// Configure initial sharded cluster +const st = new ShardingTest({shards: 2}); +const mongos = st.s; +const testDb = mongos.getDB("test"); +const coll = testDb.getCollection("foo"); + +// Enable sharding at collection 'foo' and create a new shard key +assert.commandWorked(st.s.adminCommand({enableSharding: testDb.getName()})); + +// Crate a new shard key +assert.commandWorked( + st.s.adminCommand({shardcollection: testDb.getName() + '.' + coll.getName(), key: {skey: 1}})); + +validateHiddenIndexBehaviour(); +validateOneShardKeyHiddenIndexBehaviour(); +validateDifferentHiddenIndexesBehaviour(); + +st.stop(); +})(); diff --git a/jstests/sharding/timeseries_coll_mod.js b/jstests/sharding/timeseries_coll_mod.js index 2627fa21cf4..36b33cb3e04 100644 --- a/jstests/sharding/timeseries_coll_mod.js +++ b/jstests/sharding/timeseries_coll_mod.js @@ -68,11 +68,10 @@ function runBasicTest(failPoint) { key: {[metaField]: 1}, })); - // Normal collMod commands works for the sharded time-series collection. - assert.commandWorked( - db.runCommand({collMod: collName, index: {name: indexName, hidden: true}})); - assert.commandWorked( - db.runCommand({collMod: collName, index: {name: indexName, hidden: false}})); + // Check that collMod commands works for the sharded time-series collection. + assert.commandWorked(db[collName].createIndex({'a': 1})); + assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'a_1', hidden: true}})); + assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'a_1', hidden: false}})); if (failPoint) { // Granularity update disabled for sharded time-series collection, when we're using primary diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index cedf7a39cef..4134c9bcf57 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -53,6 +53,7 @@ #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/s/collection_sharding_state.h" #include "mongo/db/s/database_sharding_state.h" +#include "mongo/db/s/shard_key_index_util.h" #include "mongo/db/server_options.h" #include "mongo/db/service_context.h" #include "mongo/db/storage/recovery_unit.h" @@ -303,6 +304,27 @@ StatusWith<std::pair<ParsedCollModRequest, BSONObj>> parseCollModRequest(Operati cmrIndex->idx = indexes[0]; } + if (cmdIndex.getHidden()) { + // Checks if it is possible to drop the shard key, so it could be possible to hide it + if (auto catalogClient = Grid::get(opCtx)->catalogClient()) { + try { + auto shardedColl = catalogClient->getCollection(opCtx, nss); + + if (isLastNonHiddenShardKeyIndex(opCtx, + coll, + coll->getIndexCatalog(), + indexName.toString(), + shardedColl.getKeyPattern().toBSON())) { + return {ErrorCodes::InvalidOptions, + "Cannot hide the only compatible index for this collection's shard " + "key"}; + } + } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>&) { + // The collection is unsharded or doesn't exist. + } + } + } + if (cmdIndex.getExpireAfterSeconds()) { parsed.numModifications++; BSONElement oldExpireSecs = cmrIndex->idx->infoObj().getField("expireAfterSeconds"); diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index 2c10eb005c5..4114c3e9aae 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -344,7 +344,7 @@ void dropReadyIndexes(OperationContext* opCtx, uassert( ErrorCodes::CannotDropShardKeyIndex, "Cannot drop the only compatible index for this collection's shard key", - !isLastShardKeyIndex( + !isLastNonHiddenShardKeyIndex( opCtx, collection, indexCatalog, indexName, collDescription.getKeyPattern())); } @@ -515,11 +515,11 @@ DropIndexesReply dropIndexes(OperationContext* opCtx, if (collDescription.isSharded()) { uassert(ErrorCodes::CannotDropShardKeyIndex, "Cannot drop the only compatible index for this collection's shard key", - !isLastShardKeyIndex(opCtx, - collection->getCollection(), - indexCatalog, - indexName, - collDescription.getKeyPattern())); + !isLastNonHiddenShardKeyIndex(opCtx, + collection->getCollection(), + indexCatalog, + indexName, + collDescription.getKeyPattern())); } auto desc = indexCatalog->findIndexByName(opCtx, indexName, includeUnfinished); diff --git a/src/mongo/db/s/shard_key_index_util.cpp b/src/mongo/db/s/shard_key_index_util.cpp index 56d6ca08656..542da1e129c 100644 --- a/src/mongo/db/s/shard_key_index_util.cpp +++ b/src/mongo/db/s/shard_key_index_util.cpp @@ -64,6 +64,10 @@ const boost::optional<ShardKeyIndex> _findShardKeyPrefixedIndex( continue; } + if (indexDescriptor->hidden()) { + continue; + } + if (isCompatibleWithShardKey(opCtx, collection, indexEntry, shardKey, requireSingleKey)) { if (!indexEntry->isMultikey(opCtx, collection)) { return ShardKeyIndex(indexDescriptor); @@ -128,11 +132,11 @@ bool isCompatibleWithShardKey(OperationContext* opCtx, return false; } -bool isLastShardKeyIndex(OperationContext* opCtx, - const CollectionPtr& collection, - const IndexCatalog* indexCatalog, - const std::string& indexName, - const BSONObj& shardKey) { +bool isLastNonHiddenShardKeyIndex(OperationContext* opCtx, + const CollectionPtr& collection, + const IndexCatalog* indexCatalog, + const std::string& indexName, + const BSONObj& shardKey) { return !_findShardKeyPrefixedIndex( opCtx, collection, indexCatalog, indexName, shardKey, false /* requireSingleKey */) .is_initialized(); diff --git a/src/mongo/db/s/shard_key_index_util.h b/src/mongo/db/s/shard_key_index_util.h index db59c57ceca..1fefa4af87e 100644 --- a/src/mongo/db/s/shard_key_index_util.h +++ b/src/mongo/db/s/shard_key_index_util.h @@ -81,6 +81,7 @@ bool isCompatibleWithShardKey(OperationContext* opCtx, * - must be prefixed by 'shardKey', and * - must not be a partial index. * - must have the simple collation. + * - must not be hidden. * * If the parameter 'requireSingleKey' is true, then this index additionally must not be * multi-key. @@ -92,13 +93,13 @@ const boost::optional<ShardKeyIndex> findShardKeyPrefixedIndex(OperationContext* bool requireSingleKey); /** - * Returns true if the given index name is the last remaining index that is compatible with the - * shard key index. + * Returns true if the given index name is the last remaining not hidden index that is compatible + * with the shard key index. */ -bool isLastShardKeyIndex(OperationContext* opCtx, - const CollectionPtr& collection, - const IndexCatalog* indexCatalog, - const std::string& indexName, - const BSONObj& shardKey); +bool isLastNonHiddenShardKeyIndex(OperationContext* opCtx, + const CollectionPtr& collection, + const IndexCatalog* indexCatalog, + const std::string& indexName, + const BSONObj& shardKey); } // namespace mongo diff --git a/src/mongo/db/s/shard_key_index_util_test.cpp b/src/mongo/db/s/shard_key_index_util_test.cpp index c9c1b4fdc23..bf1502b1ff3 100644 --- a/src/mongo/db/s/shard_key_index_util_test.cpp +++ b/src/mongo/db/s/shard_key_index_util_test.cpp @@ -203,9 +203,12 @@ TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithSingleCandidate) { createIndex(BSON("key" << BSON("x" << 1) << "name" << "x" << "v" << kIndexVersion)); + createIndex(BSON("key" << BSON("x" << 1 << "y" << 1) << "name" + << "xy" + << "v" << kIndexVersion << "hidden" << true)); - ASSERT_TRUE( - isLastShardKeyIndex(opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1))); + ASSERT_TRUE(isLastNonHiddenShardKeyIndex( + opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1))); } TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithMultipleCandidates) { @@ -219,8 +222,8 @@ TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithMultipleCandidates) { << "xy" << "v" << kIndexVersion)); - ASSERT_FALSE( - isLastShardKeyIndex(opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1))); + ASSERT_FALSE(isLastNonHiddenShardKeyIndex( + opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1))); } } // namespace |