From 51fe71a4169d8ac01dca915b482c94e228d8a746 Mon Sep 17 00:00:00 2001 From: Kaloian Manassiev Date: Mon, 15 Aug 2016 13:40:14 -0400 Subject: SERVER-25602 Make split/mergeChunks commands check the validity of input --- src/mongo/db/s/collection_metadata.cpp | 2 +- src/mongo/db/s/merge_chunks_command.cpp | 8 ++-- src/mongo/db/s/operation_sharding_state.h | 2 +- src/mongo/db/s/split_chunk_command.cpp | 73 ++++++++++++++----------------- 4 files changed, 39 insertions(+), 46 deletions(-) (limited to 'src') diff --git a/src/mongo/db/s/collection_metadata.cpp b/src/mongo/db/s/collection_metadata.cpp index ac902c3feb2..e3a576d88d3 100644 --- a/src/mongo/db/s/collection_metadata.cpp +++ b/src/mongo/db/s/collection_metadata.cpp @@ -201,7 +201,7 @@ StatusWith> CollectionMetadata::cloneSplit( // Check that the split key is valid if (!rangeContains(minKey, maxKey, split)) { return {ErrorCodes::IllegalOperation, - stream() << "cannot split chunk " << rangeToString(minKey, maxKey) << " at key " + stream() << "Cannot split chunk " << rangeToString(minKey, maxKey) << " at key " << split}; } diff --git a/src/mongo/db/s/merge_chunks_command.cpp b/src/mongo/db/s/merge_chunks_command.cpp index a309f77cb47..6081f342926 100644 --- a/src/mongo/db/s/merge_chunks_command.cpp +++ b/src/mongo/db/s/merge_chunks_command.cpp @@ -324,6 +324,9 @@ bool mergeChunks(OperationContext* txn, ChunkVersion mergeVersion = metadata->getCollVersion(); mergeVersion.incMinor(); + // Ensure that the newly applied chunks would result in a correct metadata state + auto metadataAfterMerge = uassertStatusOK(metadata->cloneMerge(minKey, maxKey, mergeVersion)); + Status applyOpsStatus = runApplyOpsCmd(txn, chunksToMerge, shardVersion, mergeVersion); if (!applyOpsStatus.isOK()) { warning() << applyOpsStatus; @@ -339,10 +342,7 @@ bool mergeChunks(OperationContext* txn, AutoGetCollection autoColl(txn, nss, MODE_IX, MODE_X); auto css = CollectionShardingState::get(txn, nss); - - std::unique_ptr cloned( - fassertStatusOK(40222, css->getMetadata()->cloneMerge(minKey, maxKey, mergeVersion))); - css->refreshMetadata(txn, std::move(cloned)); + css->refreshMetadata(txn, std::move(metadataAfterMerge)); } // diff --git a/src/mongo/db/s/operation_sharding_state.h b/src/mongo/db/s/operation_sharding_state.h index 0ef397b7e92..d83822d5696 100644 --- a/src/mongo/db/s/operation_sharding_state.h +++ b/src/mongo/db/s/operation_sharding_state.h @@ -116,7 +116,7 @@ private: void _clear(); bool _hasVersion = false; - ChunkVersion _shardVersion; + ChunkVersion _shardVersion{ChunkVersion::UNSHARDED()}; NamespaceString _ns; // This value will only be non-null if version check during the operation execution failed due diff --git a/src/mongo/db/s/split_chunk_command.cpp b/src/mongo/db/s/split_chunk_command.cpp index 059f1634168..b57170b5464 100644 --- a/src/mongo/db/s/split_chunk_command.cpp +++ b/src/mongo/db/s/split_chunk_command.cpp @@ -255,31 +255,22 @@ public: return false; } - ChunkVersion cmdVersion; - { - // Mongos >= v3.2 sends the full version, v3.0 only sends the epoch. - // TODO(SERVER-20742): Stop parsing epoch separately after 3.2. - OID cmdEpoch; - auto& oss = OperationShardingState::get(txn); - if (oss.hasShardVersion()) { - cmdVersion = oss.getShardVersion(nss); - cmdEpoch = cmdVersion.epoch(); - } else { - BSONElement epochElem(cmdObj["epoch"]); - if (epochElem.type() == jstOID) { - cmdEpoch = epochElem.OID(); - } - } - - if (cmdEpoch != shardVersion.epoch()) { - std::string msg = str::stream() << "splitChunk cannot split chunk " - << "[" << min << "," << max << "), " - << "collection may have been dropped. " - << "current epoch: " << shardVersion.epoch() - << ", cmd epoch: " << cmdEpoch; - warning() << msg; - throw SendStaleConfigException(nss.toString(), msg, cmdVersion, shardVersion); - } + const auto& oss = OperationShardingState::get(txn); + uassert(ErrorCodes::InvalidOptions, "collection version is missing", oss.hasShardVersion()); + + // Even though the splitChunk command transmits a value in the operation's shardVersion + // field, this value does not actually contain the shard version, but the global collection + // version. + ChunkVersion expectedCollectionVersion = oss.getShardVersion(nss); + if (expectedCollectionVersion.epoch() != shardVersion.epoch()) { + std::string msg = str::stream() << "splitChunk cannot split chunk " + << "[" << min << "," << max << "), " + << "collection may have been dropped. " + << "current epoch: " << shardVersion.epoch() + << ", cmd epoch: " << expectedCollectionVersion.epoch(); + warning() << msg; + throw SendStaleConfigException( + nss.toString(), msg, expectedCollectionVersion, shardVersion); } ScopedCollectionMetadata collMetadata; @@ -305,7 +296,8 @@ public: << "[" << min << "," << max << ")" << " to split, the chunk boundaries may be stale"; warning() << msg; - throw SendStaleConfigException(nss.toString(), msg, cmdVersion, shardVersion); + throw SendStaleConfigException( + nss.toString(), msg, expectedCollectionVersion, shardVersion); } log() << "splitChunk accepted at version " << shardVersion; @@ -402,6 +394,20 @@ public: preCond.append(b.obj()); } + // NOTE: The newShardVersion resulting from this split is higher than any other chunk + // version, so it's also implicitly the newCollVersion + ChunkVersion newShardVersion = collVersion; + + // Increment the minor version once, splitChunk increments once per split point + // (resulting in the correct final shard/collection version) + // + // TODO: Revisit this interface, it's a bit clunky + newShardVersion.incMinor(); + + // Ensure that the newly applied chunks would result in a correct metadata state + auto metadataAfterSplit = + uassertStatusOK(collMetadata->cloneSplit(min, max, splitKeys, newShardVersion)); + // // 4. apply the batch of updates to remote and local metadata // @@ -421,20 +427,7 @@ public: AutoGetCollection autoColl(txn, nss, MODE_IX, MODE_X); auto css = CollectionShardingState::get(txn, nss); - - // NOTE: The newShardVersion resulting from this split is higher than any other chunk - // version, so it's also implicitly the newCollVersion - ChunkVersion newShardVersion = collVersion; - - // Increment the minor version once, splitChunk increments once per split point - // (resulting in the correct final shard/collection version) - // - // TODO: Revisit this interface, it's a bit clunky - newShardVersion.incMinor(); - - std::unique_ptr cloned(fassertStatusOK( - 40221, css->getMetadata()->cloneSplit(min, max, splitKeys, newShardVersion))); - css->refreshMetadata(txn, std::move(cloned)); + css->refreshMetadata(txn, std::move(metadataAfterSplit)); } // -- cgit v1.2.1