diff options
author | Pol Pinol Castuera <pol.pinol@mongodb.com> | 2022-11-15 12:07:08 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-11-15 12:36:19 +0000 |
commit | 8e22d0a400e73070f752ce74bd60f99ec2997746 (patch) | |
tree | 860604e9c83ae101b80bdccc15959439ef398223 | |
parent | d7ae6e64f16f2b9951b2310c4ae10eaf8e594ba4 (diff) | |
download | mongo-8e22d0a400e73070f752ce74bd60f99ec2997746.tar.gz |
SERVER-68769 If the shard key index cannot be dropped, it cannot be hidden.
-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 |
7 files changed, 136 insertions, 27 deletions
diff --git a/jstests/sharding/hidden_index.js b/jstests/sharding/hidden_index.js new file mode 100644 index 00000000000..65806fcc075 --- /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_61, + * ] + */ + +(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 08e882ef64a..67be3a51bfc 100644 --- a/jstests/sharding/timeseries_coll_mod.js +++ b/jstests/sharding/timeseries_coll_mod.js @@ -60,11 +60,10 @@ function runBasicTest() { 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}})); // Granularity update works for sharded time-series collection, when we're using DDL // coordinator logic. diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 42a9b5ce52d..cd8a57def3d 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -52,6 +52,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" @@ -300,6 +301,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 ecbbf35893c..f5e76e677a5 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -349,7 +349,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())); } @@ -523,11 +523,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 = diff --git a/src/mongo/db/s/shard_key_index_util.cpp b/src/mongo/db/s/shard_key_index_util.cpp index 833d27d709a..4359d731dbb 100644 --- a/src/mongo/db/s/shard_key_index_util.cpp +++ b/src/mongo/db/s/shard_key_index_util.cpp @@ -68,6 +68,10 @@ boost::optional<ShardKeyIndex> _findShardKeyPrefixedIndex( continue; } + if (indexDescriptor->hidden()) { + continue; + } + if (isCompatibleWithShardKey( opCtx, collection, indexEntry, shardKey, requireSingleKey, errMsg)) { if (!indexEntry->isMultikey(opCtx, collection)) { @@ -179,11 +183,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, true /* requireSingleKey */) .has_value(); diff --git a/src/mongo/db/s/shard_key_index_util.h b/src/mongo/db/s/shard_key_index_util.h index c474363d8ac..c15840db109 100644 --- a/src/mongo/db/s/shard_key_index_util.h +++ b/src/mongo/db/s/shard_key_index_util.h @@ -85,6 +85,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. @@ -97,13 +98,13 @@ boost::optional<ShardKeyIndex> findShardKeyPrefixedIndex(OperationContext* opCtx std::string* errMsg = nullptr); /** - * 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 |