diff options
author | Cheahuychou Mao <cheahuychou.mao@mongodb.com> | 2020-04-23 16:01:57 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-04-29 17:40:40 +0000 |
commit | 33fc1326ee16fdf01aab21098697551f60fd05bb (patch) | |
tree | 0b36dd6cf7ff12970fba21fdb5de22f78d95a057 | |
parent | 3e49ba9571c3513f7aef3135b5923c83a76344e2 (diff) | |
download | mongo-33fc1326ee16fdf01aab21098697551f60fd05bb.tar.gz |
SERVER-47745 Make chunk query in ShardingCatalogManager compatible with chunks created in 3.4
(cherry picked from commit 55ea26e1ad7c01038b73551ba483194967567311)
-rw-r--r-- | jstests/multiVersion/chunk_operations_UUID_shard_key.js | 128 | ||||
-rw-r--r-- | jstests/multiVersion/libs/multi_cluster.js | 21 | ||||
-rw-r--r-- | src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp | 53 | ||||
-rw-r--r-- | src/mongo/db/s/config/sharding_catalog_manager_merge_chunks_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/s/catalog/type_chunk.cpp | 22 | ||||
-rw-r--r-- | src/mongo/s/catalog/type_chunk.h | 9 |
6 files changed, 227 insertions, 10 deletions
diff --git a/jstests/multiVersion/chunk_operations_UUID_shard_key.js b/jstests/multiVersion/chunk_operations_UUID_shard_key.js new file mode 100644 index 00000000000..1aca8a9e9a9 --- /dev/null +++ b/jstests/multiVersion/chunk_operations_UUID_shard_key.js @@ -0,0 +1,128 @@ +/* + * The _id of a chunk is the concatenation of its namespace and minbound. The string + * representation of the UUID type was changed from "BinData(...)" in 3.4 to "UUID(...)" + * in 3.6. This test is to verify that chunks for a sharded collection with a UUID shard key + * created in 3.4 and 3.6 can still be moved, split and merged after the cluster is + * upgraded to 4.2. + */ +(function() { + "use strict"; + + load('jstests/multiVersion/libs/multi_cluster.js'); + + // Checking UUID consistency uses cached connections, which are not valid across restarts. + TestData.skipCheckingUUIDsConsistentAcrossCluster = true; + + /* + * Creates a sharded collection with collection name 'collName', database name 'dbName', and + * shard key 'shardKey'. Inserts 'numDocs' docs with a UUID field into the collection, and + * splits the initial chunk into two. + */ + function setUpChunks(dbName, collName, shardKey, numDocs) { + const ns = dbName + "." + collName; + + jsTest.log(`Set up sharded collection ${ns} with UUID shard key`); + assert.commandWorked(st.s.adminCommand({shardCollection: ns, key: shardKey})); + + jsTest.log(`Insert docs for ${ns}`); + // This is necessary as splitFind does not work for empty chunks. + let bulk = st.s.getCollection(ns).initializeUnorderedBulkOp(); + for (let i = 0; i < numDocs; ++i) { + bulk.insert({x: UUID()}); + } + assert.commandWorked(bulk.execute()); + + jsTest.log(`Split the initial chunk for ${ns}`); + assert.commandWorked(st.splitFind(ns, {x: MinKey})); + assert.eq(2, st.s.getDB("config").chunks.count({ns: ns})); + + return ns; + } + + /* + * Upgrades the entire cluster to the given binVersion and waits for config server and shards + * to become available and for the replica set monitors on the mongos and each shard to reflect + * the state of all shards. Then, runs setFCV to the given FCV version. + */ + function upgradeCluster(binVersion, featureCompatibilityVersion) { + st.stopBalancer(); + st.upgradeCluster(binVersion, { + upgradeMongos: true, + upgradeConfigs: true, + upgradeShards: true, + waitUntilStable: true + }); + st.s.adminCommand({setFeatureCompatibilityVersion: featureCompatibilityVersion}); + } + + /* + * Selects the chunk with non-MinKey min to do operations on. Assumes that all chunks are on + * shard0 since it is the primary shard. Runs moveChunk, splitChunk and mergeChunk and asserts + * the commands work. + */ + function testChunkOperations(ns) { + // Make sure there is at least one chunk whose min is not MinKey. + const numOriginalChunks = st.s.getDB("config").chunks.count({ns: ns}); + assert.gt(numOriginalChunks, 1); + + // Select the chunk with max of MaxKey. + const chunkToMove = st.s.getDB("config").chunks.findOne( + {ns: ns, shard: st.shard0.shardName, min: {$ne: {x: MinKey}}, max: {x: MaxKey}}); + jsTest.log("chunkToMove " + tojson(chunkToMove)); + + jsTest.log("Move the chunk"); + assert.commandWorked( + st.s.adminCommand({moveChunk: ns, find: chunkToMove.min, to: st.shard1.shardName})); + + jsTest.log("Split the chunk"); + assert.commandWorked(st.splitFind(ns, chunkToMove.min)); + assert.eq(numOriginalChunks + 1, st.s.getDB("config").chunks.count({ns: ns})); + + jsTest.log("Merge the resulting chunks"); + assert.commandWorked( + st.s.adminCommand({mergeChunks: ns, bounds: [chunkToMove.min, chunkToMove.max]})); + assert.eq(numOriginalChunks, st.s.getDB("config").chunks.count({ns: ns})); + } + + jsTest.log("Start a \"3.4\" sharded cluster"); + + const st = new ShardingTest({ + shards: 2, + mongos: 1, + config: 1, + other: { + mongosOptions: {binVersion: "3.4"}, + configOptions: {binVersion: "3.4"}, + shardOptions: {binVersion: "3.4"}, + rsOptions: {binVersion: "3.4"}, + rs: true, + } + }); + const kDbName = "foo"; + const kCollName = "bar"; + const kShardKey = {x: 1}; + const kNumDocs = 100; + + assert.commandWorked(st.s.adminCommand({enableSharding: kDbName})); + st.ensurePrimaryShard(kDbName, st.shard0.shardName); + const ns34 = setUpChunks(kDbName, kCollName + "-34", kShardKey, kNumDocs); + + jsTest.log("Upgrade the cluster to 3.6"); + upgradeCluster("last-stable", "3.6"); + const ns36 = setUpChunks(kDbName, kCollName + "-36", kShardKey, kNumDocs); + + jsTest.log("Upgrade the cluster to 4.0"); + upgradeCluster("latest", "4.0"); + const ns40 = setUpChunks(kDbName, kCollName + "-40", kShardKey, kNumDocs); + + jsTest.log("Perform operations on chunks created when the cluster was in 3.4"); + testChunkOperations(ns34); + + jsTest.log("Perform operations on chunks created when the cluster is in 3.6"); + testChunkOperations(ns36); + + jsTest.log("Perform operations on chunks created when the cluster was in 4.0"); + testChunkOperations(ns40); + + st.stop(); +})(); diff --git a/jstests/multiVersion/libs/multi_cluster.js b/jstests/multiVersion/libs/multi_cluster.js index 2937d0d4d6d..0cea472ea50 100644 --- a/jstests/multiVersion/libs/multi_cluster.js +++ b/jstests/multiVersion/libs/multi_cluster.js @@ -2,6 +2,9 @@ // MultiVersion utility functions for clusters // +load('jstests/multiVersion/libs/multi_rs.js'); // For upgradeSet. +load('jstests/replsets/rslib.js'); // For awaitRSClientHosts. + /** * Restarts the specified binaries in options with the specified binVersion. * Note: this does not perform any upgrade operations. @@ -23,6 +26,8 @@ ShardingTest.prototype.upgradeCluster = function(binVersion, options) { options.upgradeConfigs = true; if (options.upgradeMongos == undefined) options.upgradeMongos = true; + if (options.waitUntilStable == undefined) + options.waitUntilStable = false; var upgradedSingleShards = []; @@ -86,6 +91,22 @@ ShardingTest.prototype.upgradeCluster = function(binVersion, options) { this.config = this.s.getDB("config"); this.admin = this.s.getDB("admin"); } + + if (options.waitUntilStable) { + // Wait for the config server and shards to become available. + this.configRS.awaitSecondaryNodes(); + let shardPrimaries = []; + for (let rs of this._rs) { + rs.test.awaitSecondaryNodes(); + shardPrimaries.push(rs.test.getPrimary()); + } + + // Wait for the ReplicaSetMonitor on mongoS and each shard to reflect the state of all + // shards. + for (let client of[...this._mongos, ...shardPrimaries]) { + awaitRSClientHosts(client, shardPrimaries, {ok: true, ismaster: true}); + } + } }; ShardingTest.prototype.restartMongoses = function() { diff --git a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp index a7e0be3a172..3133094adb4 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_chunk_operations.cpp @@ -196,7 +196,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, op.append("ns", ChunkType::ConfigNS.ns()); BSONObjBuilder n(op.subobjStart("o")); - n.append(ChunkType::name(), ChunkType::genID(nss, migratedChunk.getMin())); + n.append(ChunkType::name(), migratedChunk.getName()); migratedChunk.getVersion().appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), migratedChunk.getMin()); @@ -206,7 +206,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, n.done(); BSONObjBuilder q(op.subobjStart("o2")); - q.append(ChunkType::name(), ChunkType::genID(nss, migratedChunk.getMin())); + q.append(ChunkType::name(), migratedChunk.getName()); q.done(); updates.append(op.obj()); @@ -220,7 +220,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, op.append("ns", ChunkType::ConfigNS.ns()); BSONObjBuilder n(op.subobjStart("o")); - n.append(ChunkType::name(), ChunkType::genID(nss, controlChunk->getMin())); + n.append(ChunkType::name(), controlChunk->getName()); controlChunk->getVersion().appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), controlChunk->getMin()); @@ -231,7 +231,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, n.done(); BSONObjBuilder q(op.subobjStart("o2")); - q.append(ChunkType::name(), ChunkType::genID(nss, controlChunk->getMin())); + q.append(ChunkType::name(), controlChunk->getName()); q.done(); updates.append(op.obj()); @@ -376,6 +376,9 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, auto newChunkBounds(splitPoints); newChunkBounds.push_back(range.getMax()); + auto shouldTakeOriginalChunkID = true; + std::string chunkID; + BSONArrayBuilder updates; for (const auto& endKey : newChunkBounds) { @@ -421,6 +424,13 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, // splits only update the 'minor' portion of version currentMaxVersion.incMinor(); + // First chunk takes ID of the original chunk and all other chunks get new IDs. This occurs + // because we perform an update operation below (with upsert true). Keeping the original ID + // ensures we overwrite the old chunk (before the split) without having to perform a delete. + chunkID = shouldTakeOriginalChunkID ? origChunk.getValue().getName() + : ChunkType::genID(nss, startKey); + shouldTakeOriginalChunkID = false; + // build an update operation against the chunks collection of the config database // with upsert true BSONObjBuilder op; @@ -430,7 +440,7 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, // add the modified (new) chunk information as the update object BSONObjBuilder n(op.subobjStart("o")); - n.append(ChunkType::name(), ChunkType::genID(nss, startKey)); + n.append(ChunkType::name(), chunkID); currentMaxVersion.appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), startKey); @@ -447,7 +457,7 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, // add the chunk's _id as the query part of the update statement BSONObjBuilder q(op.subobjStart("o2")); - q.append(ChunkType::name(), ChunkType::genID(nss, startKey)); + q.append(ChunkType::name(), chunkID); q.done(); updates.append(op.obj()); @@ -593,6 +603,17 @@ Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx, for (size_t i = 1; i < chunkBoundaries.size(); ++i) { itChunk.setMin(itChunk.getMax()); + // Read the original chunk from disk to lookup that chunk's _id. Propagate the _id + // because in clusters where the shard key is a UUID and the chunk was last written + // in v3.4, the chunk entry's _id will be in the "BinData()" form, and calling + // ChunkType::genID would try to do the update with the _id in the "UUID()" form + // (because UUID::toString changed in v3.6), which wouldn't match any documents. + auto itOrigChunk = _findChunkOnConfig(opCtx, nss, itChunk.getMin()); + if (!itOrigChunk.isOK()) { + return itOrigChunk.getStatus(); + } + itChunk.setName(itOrigChunk.getValue().getName()); + // Ensure the chunk boundaries are strictly increasing if (chunkBoundaries[i].woCompare(itChunk.getMin()) <= 0) { return { @@ -748,6 +769,13 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkMigration( newMigratedChunk.setVersion(ChunkVersion( currentCollectionVersion.majorVersion() + 1, 0, currentCollectionVersion.epoch())); + // Propagate the _id from the original chunk because in clusters where the + // shard key is a UUID and the chunk was last written in v3.4, the chunk entry's + // _id will be in the "BinData()" form, and calling ChunkType::genID would try to + // do the update with the _id in the "UUID()" form (because UUID::toString changed + // in v3.6), which wouldn't match any documents. + newMigratedChunk.setName(origChunk.getValue().getName()); + // Copy the complete history. auto newHistory = origChunk.getValue().getHistory(); const int kHistorySecs = 10; @@ -800,6 +828,13 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkMigration( // FCV 3.6 does not have the history field in the persisted metadata newControlChunk->setHistory({}); } + + // Propagate the _id from the original chunk because in clusters where the + // shard key is a UUID and the chunk was last written in v3.4, the chunk entry's + // _id will be in the "BinData()" form, and calling ChunkType::genID would try to + // do the update with the _id in the "UUID()" form (because UUID::toString changed + // in v3.6), which wouldn't match any documents. + newControlChunk->setName(origControlChunk.getValue().getName()); } auto command = makeCommitChunkTransactionCommand( @@ -840,7 +875,7 @@ StatusWith<ChunkType> ShardingCatalogManager::_findChunkOnConfig(OperationContex ReadPreferenceSetting{ReadPreference::PrimaryOnly}, repl::ReadConcernLevel::kLocalReadConcern, ChunkType::ConfigNS, - BSON(ChunkType::name << ChunkType::genID(nss, key)), + BSON(ChunkType::ns(nss.ns()) << ChunkType::min(key)), BSONObj(), 1); @@ -851,7 +886,9 @@ StatusWith<ChunkType> ShardingCatalogManager::_findChunkOnConfig(OperationContex const auto origChunks = std::move(findResponse.getValue().docs); if (origChunks.size() != 1) { return {ErrorCodes::IncompatibleShardingMetadata, - str::stream() << "Tried to find the chunk for '" << ChunkType::genID(nss, key) + str::stream() << "Tried to find the chunk for namespace " << nss.ns() + << " and min key " + << key.toString() << ", but found no chunks"}; } diff --git a/src/mongo/db/s/config/sharding_catalog_manager_merge_chunks_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_merge_chunks_test.cpp index 08dc37fccfa..2484319ff5e 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_merge_chunks_test.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_merge_chunks_test.cpp @@ -401,7 +401,7 @@ TEST_F(MergeChunkTest, NonMatchingEpochsOfChunkAndRequestErrors) { ASSERT_EQ(ErrorCodes::StaleEpoch, mergeStatus); } -TEST_F(MergeChunkTest, MergeAlreadyHappenedFailsPrecondition) { +TEST_F(MergeChunkTest, MergeAlreadyHappenedFails) { ChunkType chunk; chunk.setNS(kNamespace); @@ -434,7 +434,7 @@ TEST_F(MergeChunkTest, MergeAlreadyHappenedFailsPrecondition) { Timestamp validAfter{1}; - ASSERT_EQ(ErrorCodes::BadValue, + ASSERT_EQ(ErrorCodes::IncompatibleShardingMetadata, ShardingCatalogManager::get(operationContext()) ->commitChunkMerge(operationContext(), kNamespace, diff --git a/src/mongo/s/catalog/type_chunk.cpp b/src/mongo/s/catalog/type_chunk.cpp index 4965975b8a0..3417050b814 100644 --- a/src/mongo/s/catalog/type_chunk.cpp +++ b/src/mongo/s/catalog/type_chunk.cpp @@ -210,6 +210,19 @@ StatusWith<ChunkType> ChunkType::fromConfigBSON(const BSONObj& source) { ChunkType chunk; { + std::string chunkID; + Status status = bsonExtractStringField(source, name.name(), &chunkID); + if (status.isOK()) { + chunk._id = chunkID; + } else if (status == ErrorCodes::NoSuchKey) { + // Ignore NoSuchKey because when chunks are sent in commands they are not required to + // include the _id. + } else { + return status; + } + } + + { std::string chunkNS; Status status = bsonExtractStringField(source, ns.name(), &chunkNS); if (!status.isOK()) @@ -372,11 +385,20 @@ BSONObj ChunkType::toShardBSON() const { } std::string ChunkType::getName() const { + if (_id) { + // This chunk already has an id so return it. + return *_id; + } + invariant(_nss); invariant(_min); return genID(*_nss, *_min); } +void ChunkType::setName(const std::string& id) { + _id = id; +} + void ChunkType::setNS(const NamespaceString& nss) { invariant(nss.isValid()); _nss = nss; diff --git a/src/mongo/s/catalog/type_chunk.h b/src/mongo/s/catalog/type_chunk.h index ed15f2ff578..06048736c22 100644 --- a/src/mongo/s/catalog/type_chunk.h +++ b/src/mongo/s/catalog/type_chunk.h @@ -219,6 +219,8 @@ public: std::string getName() const; + void setName(const std::string& id); + /** * Getters and setters. */ @@ -290,6 +292,13 @@ public: private: // Convention: (M)andatory, (O)ptional, (S)pecial; (C)onfig, (S)hard. + // (O)(C) _id of the config.chunks doc for this chunk + // UUID::toString changed between v3.4 and v3.6, so for collections where the + // shard key contains a UUID, getName will produce a different _id string (via + // genID) in v3.6 than v3.4. Since _id is immutable, this was added to the + // ChunkType API to allow commitChunk{Merge,Migration,Split} to propagate the _id + // of a chunk written in v3.4 to the new chunk(s) they write. + boost::optional<std::string> _id; // (O)(C) collection this chunk is in boost::optional<NamespaceString> _nss; // (M)(C)(S) first key of the range, inclusive |