diff options
author | Kaloian Manassiev <kaloian.manassiev@mongodb.com> | 2018-06-20 19:34:16 -0400 |
---|---|---|
committer | Kaloian Manassiev <kaloian.manassiev@mongodb.com> | 2018-06-22 14:04:18 -0400 |
commit | 5dabb36c0b930f03f8da5cc1b572f3edcf4cff58 (patch) | |
tree | 8360063842d5e14cc82b21d370dcde8499df10b9 /src | |
parent | cfb0b9ff7ca1a20c3c37edd2baf6e5c509c65aca (diff) | |
download | mongo-5dabb36c0b930f03f8da5cc1b572f3edcf4cff58.tar.gz |
SERVER-35691 Cleanup the ChunkVersion serialization/deserialization code
Diffstat (limited to 'src')
35 files changed, 265 insertions, 430 deletions
diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index 9865c7bed1a..fc0c017d160 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -145,7 +145,7 @@ void serializeReply(OperationContext* opCtx, error.append("code", int(ErrorCodes::StaleShardVersion)); // Different from exception! { BSONObjBuilder errInfo(error.subobjStart("errInfo")); - staleInfo->getVersionWanted().addToBSON(errInfo, "vWanted"); + staleInfo->getVersionWanted().appendLegacyWithField(&errInfo, "vWanted"); } } else { error.append("code", int(status.code())); diff --git a/src/mongo/db/s/balancer/migration_manager_test.cpp b/src/mongo/db/s/balancer/migration_manager_test.cpp index 5aae83503a2..4f83a2a5487 100644 --- a/src/mongo/db/s/balancer/migration_manager_test.cpp +++ b/src/mongo/db/s/balancer/migration_manager_test.cpp @@ -218,7 +218,7 @@ void MigrationManagerTest::setUpMigration(const ChunkType& chunk, const ShardId& builder.append(MigrationType::max(), chunk.getMax()); builder.append(MigrationType::toShard(), toShard.toString()); builder.append(MigrationType::fromShard(), chunk.getShard().toString()); - chunk.getVersion().appendWithFieldForCommands(&builder, "chunkVersion"); + chunk.getVersion().appendWithField(&builder, "chunkVersion"); MigrationType migrationType = assertGet(MigrationType::fromBSON(builder.obj())); ASSERT_OK(catalogClient()->insertConfigDocument(operationContext(), diff --git a/src/mongo/db/s/balancer/scoped_migration_request_test.cpp b/src/mongo/db/s/balancer/scoped_migration_request_test.cpp index 1d9b7ba6c7d..b72534b31a0 100644 --- a/src/mongo/db/s/balancer/scoped_migration_request_test.cpp +++ b/src/mongo/db/s/balancer/scoped_migration_request_test.cpp @@ -98,7 +98,7 @@ MigrateInfo makeMigrateInfo() { chunkBuilder.append(ChunkType::ns(), kNs); chunkBuilder.append(ChunkType::min(), kMin); chunkBuilder.append(ChunkType::max(), kMax); - kChunkVersion.appendForChunk(&chunkBuilder); + kChunkVersion.appendLegacyWithField(&chunkBuilder, ChunkType::lastmod()); chunkBuilder.append(ChunkType::shard(), kFromShard.toString()); ChunkType chunkType = assertGet(ChunkType::fromConfigBSON(chunkBuilder.obj())); diff --git a/src/mongo/db/s/balancer/type_migration.cpp b/src/mongo/db/s/balancer/type_migration.cpp index e6341b77685..bcd5f8ffe56 100644 --- a/src/mongo/db/s/balancer/type_migration.cpp +++ b/src/mongo/db/s/balancer/type_migration.cpp @@ -99,8 +99,7 @@ StatusWith<MigrationType> MigrationType::fromBSON(const BSONObj& source) { } { - auto chunkVersionStatus = - ChunkVersion::parseFromBSONWithFieldForCommands(source, kChunkVersion); + auto chunkVersionStatus = ChunkVersion::parseWithField(source, kChunkVersion); if (!chunkVersionStatus.isOK()) return chunkVersionStatus.getStatus(); migrationType._chunkVersion = chunkVersionStatus.getValue(); @@ -130,7 +129,7 @@ BSONObj MigrationType::toBSON() const { builder.append(fromShard.name(), _fromShard.toString()); builder.append(toShard.name(), _toShard.toString()); - _chunkVersion.appendWithFieldForCommands(&builder, kChunkVersion); + _chunkVersion.appendWithField(&builder, kChunkVersion); builder.append(waitForDelete.name(), _waitForDelete); return builder.obj(); diff --git a/src/mongo/db/s/balancer/type_migration_test.cpp b/src/mongo/db/s/balancer/type_migration_test.cpp index 8d5d7cd07db..4b14d8cd018 100644 --- a/src/mongo/db/s/balancer/type_migration_test.cpp +++ b/src/mongo/db/s/balancer/type_migration_test.cpp @@ -56,7 +56,7 @@ TEST(MigrationTypeTest, ConvertFromMigrationInfo) { chunkBuilder.append(ChunkType::ns(), kNs); chunkBuilder.append(ChunkType::min(), kMin); chunkBuilder.append(ChunkType::max(), kMax); - version.appendForChunk(&chunkBuilder); + version.appendLegacyWithField(&chunkBuilder, ChunkType::lastmod()); chunkBuilder.append(ChunkType::shard(), kFromShard.toString()); ChunkType chunkType = assertGet(ChunkType::fromConfigBSON(chunkBuilder.obj())); @@ -72,7 +72,7 @@ TEST(MigrationTypeTest, ConvertFromMigrationInfo) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); builder.append(MigrationType::waitForDelete(), kWaitForDelete); BSONObj obj = builder.obj(); @@ -90,7 +90,7 @@ TEST(MigrationTypeTest, FromAndToBSON) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); builder.append(MigrationType::waitForDelete(), kWaitForDelete); BSONObj obj = builder.obj(); @@ -107,7 +107,7 @@ TEST(MigrationTypeTest, MissingRequiredNamespaceField) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); BSONObj obj = builder.obj(); @@ -124,7 +124,7 @@ TEST(MigrationTypeTest, MissingRequiredMinField) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); BSONObj obj = builder.obj(); @@ -141,7 +141,7 @@ TEST(MigrationTypeTest, MissingRequiredMaxField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); BSONObj obj = builder.obj(); @@ -158,7 +158,7 @@ TEST(MigrationTypeTest, MissingRequiredFromShardField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::max(), kMax); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); BSONObj obj = builder.obj(); @@ -175,7 +175,7 @@ TEST(MigrationTypeTest, MissingRequiredToShardField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); - version.appendWithFieldForCommands(&builder, "chunkVersion"); + version.appendWithField(&builder, "chunkVersion"); BSONObj obj = builder.obj(); diff --git a/src/mongo/db/s/collection_metadata.cpp b/src/mongo/db/s/collection_metadata.cpp index 7e4b140d940..fa2455ee981 100644 --- a/src/mongo/db/s/collection_metadata.cpp +++ b/src/mongo/db/s/collection_metadata.cpp @@ -95,8 +95,8 @@ Status CollectionMetadata::checkChunkIsValid(const ChunkType& chunk) const { } void CollectionMetadata::toBSONBasic(BSONObjBuilder& bb) const { - _cm->getVersion().addToBSON(bb, "collVersion"); - getShardVersion().addToBSON(bb, "shardVersion"); + _cm->getVersion().appendLegacyWithField(&bb, "collVersion"); + getShardVersion().appendLegacyWithField(&bb, "shardVersion"); bb.append("keyPattern", _cm->getShardKeyPattern().toBSON()); } diff --git a/src/mongo/db/s/config/configsvr_drop_collection_command.cpp b/src/mongo/db/s/config/configsvr_drop_collection_command.cpp index 3663903cb6b..432c28fc442 100644 --- a/src/mongo/db/s/config/configsvr_drop_collection_command.cpp +++ b/src/mongo/db/s/config/configsvr_drop_collection_command.cpp @@ -202,7 +202,7 @@ private: // not sharded. Collections residing on the config server are never sharded so do not // send the shard version. if (shardId != shardRegistry->getConfigShard()->getId()) { - ChunkVersion::UNSHARDED().appendForCommands(&builder); + ChunkVersion::UNSHARDED().appendToCommand(&builder); } if (!opCtx->getWriteConcern().usedDefault) { 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 f7734383e6f..bdbc7226e19 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 @@ -60,12 +60,13 @@ MONGO_FAIL_POINT_DEFINE(migrationCommitVersionError); /** * Append min, max and version information from chunk to the buffer for logChange purposes. */ -void appendShortVersion(BufBuilder* b, const ChunkType& chunk) { - BSONObjBuilder bb(*b); +void appendShortVersion(BufBuilder* out, const ChunkType& chunk) { + BSONObjBuilder bb(*out); bb.append(ChunkType::min(), chunk.getMin()); bb.append(ChunkType::max(), chunk.getMax()); - if (chunk.isVersionSet()) - chunk.getVersion().addToBSON(bb, ChunkType::lastmod()); + if (chunk.isVersionSet()) { + chunk.getVersion().appendLegacyWithField(&bb, ChunkType::lastmod()); + } bb.done(); } @@ -187,7 +188,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, BSONObjBuilder n(op.subobjStart("o")); n.append(ChunkType::name(), ChunkType::genID(nss, migratedChunk.getMin())); - migratedChunk.getVersion().addToBSON(n, ChunkType::lastmod()); + migratedChunk.getVersion().appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), migratedChunk.getMin()); n.append(ChunkType::max(), migratedChunk.getMax()); @@ -211,7 +212,7 @@ BSONObj makeCommitChunkTransactionCommand(const NamespaceString& nss, BSONObjBuilder n(op.subobjStart("o")); n.append(ChunkType::name(), ChunkType::genID(nss, controlChunk->getMin())); - controlChunk->getVersion().addToBSON(n, ChunkType::lastmod()); + controlChunk->getVersion().appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), controlChunk->getMin()); n.append(ChunkType::max(), controlChunk->getMax()); @@ -300,7 +301,8 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, return {ErrorCodes::IllegalOperation, errmsg}; } - ChunkVersion collVersion = ChunkVersion::fromBSON(chunksVector.front(), ChunkType::lastmod()); + ChunkVersion collVersion = uassertStatusOK( + ChunkVersion::parseLegacyWithField(chunksVector.front(), ChunkType::lastmod())); // Return an error if collection epoch does not match epoch of request. if (collVersion.epoch() != requestEpoch) { @@ -380,7 +382,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)); - currentMaxVersion.addToBSON(n, ChunkType::lastmod()); + currentMaxVersion.appendLegacyWithField(&n, ChunkType::lastmod()); n.append(ChunkType::ns(), nss.ns()); n.append(ChunkType::min(), startKey); n.append(ChunkType::max(), endKey); @@ -445,7 +447,7 @@ Status ShardingCatalogManager::commitChunkSplit(OperationContext* opCtx, BSONObjBuilder b(logDetail.subobjStart("before")); b.append(ChunkType::min(), range.getMin()); b.append(ChunkType::max(), range.getMax()); - collVersion.addToBSON(b, ChunkType::lastmod()); + collVersion.appendLegacyWithField(&b, ChunkType::lastmod()); } if (newChunks.size() == 2) { @@ -516,7 +518,8 @@ Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx, return {ErrorCodes::IllegalOperation, "collection does not exist, isn't sharded, or has no chunks"}; - ChunkVersion collVersion = ChunkVersion::fromBSON(chunksVector.front(), ChunkType::lastmod()); + ChunkVersion collVersion = uassertStatusOK( + ChunkVersion::parseLegacyWithField(chunksVector.front(), ChunkType::lastmod())); // Return an error if epoch of chunk does not match epoch of request if (collVersion.epoch() != requestEpoch) { @@ -580,8 +583,8 @@ Status ShardingCatalogManager::commitChunkMerge(OperationContext* opCtx, b.append(chunkToMerge.toConfigBSON()); } } - collVersion.addToBSON(logDetail, "prevShardVersion"); - mergeVersion.addToBSON(logDetail, "mergedVersion"); + collVersion.appendLegacyWithField(&logDetail, "prevShardVersion"); + mergeVersion.appendLegacyWithField(&logDetail, "mergedVersion"); Grid::get(opCtx) ->catalogClient() @@ -751,9 +754,9 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkMigration( } BSONObjBuilder result; - newMigratedChunk.getVersion().appendWithFieldForCommands(&result, "migratedChunkVersion"); + newMigratedChunk.getVersion().appendWithField(&result, "migratedChunkVersion"); if (controlChunk) { - newControlChunk->getVersion().appendWithFieldForCommands(&result, "controlChunkVersion"); + newControlChunk->getVersion().appendWithField(&result, "controlChunkVersion"); } return result.obj(); diff --git a/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp index 901c6a5b104..8b68f86246e 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_commit_chunk_migration_test.cpp @@ -104,11 +104,11 @@ TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandWithCtl) { // Verify the versions returned match expected values. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "migratedChunkVersion"); + auto mver = ChunkVersion::parseWithField(versions, "migratedChunkVersion"); ASSERT_OK(mver.getStatus()); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, origVersion.epoch()), mver.getValue()); - auto cver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "controlChunkVersion"); + auto cver = ChunkVersion::parseWithField(versions, "controlChunkVersion"); ASSERT_OK(cver.getStatus()); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 1, origVersion.epoch()), cver.getValue()); @@ -174,11 +174,11 @@ TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandNoCtl) { // Verify the version returned matches expected value. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "migratedChunkVersion"); + auto mver = ChunkVersion::parseWithField(versions, "migratedChunkVersion"); ASSERT_OK(mver.getStatus()); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, origVersion.epoch()), mver.getValue()); - auto cver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "controlChunkVersion"); + auto cver = ChunkVersion::parseWithField(versions, "controlChunkVersion"); ASSERT_NOT_OK(cver.getStatus()); // Verify the chunk ended up in the right shard, and version matches the value returned. @@ -235,11 +235,11 @@ TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandNoCtlTrimHistory) { // Verify the version returned matches expected value. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "migratedChunkVersion"); + auto mver = ChunkVersion::parseWithField(versions, "migratedChunkVersion"); ASSERT_OK(mver.getStatus()); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, origVersion.epoch()), mver.getValue()); - auto cver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "controlChunkVersion"); + auto cver = ChunkVersion::parseWithField(versions, "controlChunkVersion"); ASSERT_NOT_OK(cver.getStatus()); // Verify the chunk ended up in the right shard, and version matches the value returned. @@ -505,7 +505,7 @@ TEST_F(CommitChunkMigrate, CommitWithLastChunkOnShardShouldNotAffectOtherChunks) // Verify the versions returned match expected values. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseFromBSONWithFieldForCommands(versions, "migratedChunkVersion"); + auto mver = ChunkVersion::parseWithField(versions, "migratedChunkVersion"); ASSERT_OK(mver.getStatus()); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, origVersion.epoch()), mver.getValue()); diff --git a/src/mongo/db/s/operation_sharding_state.cpp b/src/mongo/db/s/operation_sharding_state.cpp index 5f855ba10d2..5527d066453 100644 --- a/src/mongo/db/s/operation_sharding_state.cpp +++ b/src/mongo/db/s/operation_sharding_state.cpp @@ -75,17 +75,7 @@ void OperationShardingState::initializeClientRoutingVersions(NamespaceString nss const auto shardVersionElem = cmdObj.getField(ChunkVersion::kShardVersionField); if (!shardVersionElem.eoo()) { - uassert(ErrorCodes::BadValue, - str::stream() << "expected shardVersion element to be an array, got " - << shardVersionElem, - shardVersionElem.type() == BSONType::Array); - const BSONArray versionArr(shardVersionElem.Obj()); - - bool canParse; - ChunkVersion shardVersion = ChunkVersion::fromBSON(versionArr, &canParse); - uassert(ErrorCodes::BadValue, - str::stream() << "could not parse shardVersion from field " << versionArr, - canParse); + auto shardVersion = uassertStatusOK(ChunkVersion::parseFromCommand(cmdObj)); if (nss.isSystemDotIndexes()) { _shardVersions[nss.ns()] = ChunkVersion::IGNORED(); diff --git a/src/mongo/db/s/set_shard_version_command.cpp b/src/mongo/db/s/set_shard_version_command.cpp index 967332fabc2..acfd347345c 100644 --- a/src/mongo/db/s/set_shard_version_command.cpp +++ b/src/mongo/db/s/set_shard_version_command.cpp @@ -48,14 +48,11 @@ #include "mongo/db/s/sharding_state.h" #include "mongo/s/client/shard_registry.h" #include "mongo/s/grid.h" +#include "mongo/s/request_types/set_shard_version_request.h" #include "mongo/util/log.h" #include "mongo/util/stringutils.h" namespace mongo { - -using std::string; -using str::stream; - namespace { class SetShardVersion : public ErrmsgCommandDeprecated { @@ -89,7 +86,7 @@ public: bool errmsgRun(OperationContext* opCtx, const std::string&, const BSONObj& cmdObj, - string& errmsg, + std::string& errmsg, BSONObjBuilder& result) { uassert(ErrorCodes::IllegalOperation, "can't issue setShardVersion from 'eval'", @@ -197,8 +194,8 @@ public: nss.isValid()); // Validate chunk version parameter. - const ChunkVersion requestedVersion = - uassertStatusOK(ChunkVersion::parseFromBSONForSetShardVersion(cmdObj)); + const ChunkVersion requestedVersion = uassertStatusOK( + ChunkVersion::parseLegacyWithField(cmdObj, SetShardVersionRequest::kVersion)); // Step 4 @@ -210,7 +207,7 @@ public: // as UNSHARDED is the legacy way to achieve this purpose. const auto connectionVersion = (connectionVersionOrNotSet ? *connectionVersionOrNotSet : ChunkVersion::UNSHARDED()); - connectionVersion.addToBSON(result, "oldVersion"); + connectionVersion.appendLegacyWithField(&result, "oldVersion"); { boost::optional<AutoGetDb> autoDb; @@ -273,7 +270,7 @@ public: if (!authoritative) { result.appendBool("need_authoritative", true); result.append("ns", nss.ns()); - collectionShardVersion.addToBSON(result, "globalVersion"); + collectionShardVersion.appendLegacyWithField(&result, "globalVersion"); errmsg = "dropping needs to be authoritative"; return false; } @@ -290,8 +287,8 @@ public: errmsg = str::stream() << "this connection already had a newer version " << "of collection '" << nss.ns() << "'"; result.append("ns", nss.ns()); - requestedVersion.addToBSON(result, "newVersion"); - collectionShardVersion.addToBSON(result, "globalVersion"); + requestedVersion.appendLegacyWithField(&result, "newVersion"); + collectionShardVersion.appendLegacyWithField(&result, "globalVersion"); return false; } @@ -310,8 +307,8 @@ public: errmsg = str::stream() << "shard global version for collection is higher " << "than trying to set to '" << nss.ns() << "'"; result.append("ns", nss.ns()); - requestedVersion.addToBSON(result, "version"); - collectionShardVersion.addToBSON(result, "globalVersion"); + requestedVersion.appendLegacyWithField(&result, "version"); + collectionShardVersion.appendLegacyWithField(&result, "globalVersion"); result.appendBool("reloadConfig", true); return false; } @@ -367,8 +364,8 @@ public: warning() << errmsg; result.append("ns", nss.ns()); - requestedVersion.addToBSON(result, "version"); - currVersion.addToBSON(result, "globalVersion"); + requestedVersion.appendLegacyWithField(&result, "version"); + currVersion.appendLegacyWithField(&result, "globalVersion"); result.appendBool("reloadConfig", true); return false; @@ -386,7 +383,7 @@ public: // version reload. result.append("ns", nss.ns()); - currVersion.addToBSON(result, "globalVersion"); + currVersion.appendLegacyWithField(&result, "globalVersion"); // If this was a reset of a collection or the last chunk moved out, inform mongos to // do a full reload. @@ -394,11 +391,11 @@ public: result.appendBool("reloadConfig", true); // Zero-version also needed to trigger full mongos reload, sadly // TODO: Make this saner, and less impactful (full reload on last chunk is bad) - ChunkVersion(0, 0, OID()).addToBSON(result, "version"); + ChunkVersion(0, 0, OID()).appendLegacyWithField(&result, "version"); // For debugging - requestedVersion.addToBSON(result, "origVersion"); + requestedVersion.appendLegacyWithField(&result, "origVersion"); } else { - requestedVersion.addToBSON(result, "version"); + requestedVersion.appendLegacyWithField(&result, "version"); } return false; diff --git a/src/mongo/db/s/shard_metadata_util.cpp b/src/mongo/db/s/shard_metadata_util.cpp index b5669bdc004..28384a70a85 100644 --- a/src/mongo/db/s/shard_metadata_util.cpp +++ b/src/mongo/db/s/shard_metadata_util.cpp @@ -366,7 +366,7 @@ Status updateShardChunks(OperationContext* opCtx, * */ for (auto& chunk : chunks) { - invariant(chunk.getVersion().hasEqualEpoch(currEpoch)); + invariant(chunk.getVersion().epoch() == currEpoch); // Delete any overlapping chunk ranges. Overlapping chunks will have a min value // ("_id") between (chunk.min, chunk.max]. diff --git a/src/mongo/db/s/shard_metadata_util_test.cpp b/src/mongo/db/s/shard_metadata_util_test.cpp index 8c650e17531..b6a397f26df 100644 --- a/src/mongo/db/s/shard_metadata_util_test.cpp +++ b/src/mongo/db/s/shard_metadata_util_test.cpp @@ -287,7 +287,7 @@ TEST_F(ShardMetadataUtilTest, UpdateWithWriteNewChunks) { subMax.append("a", 10000); } splitChunkOneBuilder.append(ChunkType::shard(), lastChunk.getShard().toString()); - collVersion.appendForChunk(&splitChunkOneBuilder); + collVersion.appendLegacyWithField(&splitChunkOneBuilder, ChunkType::lastmod()); ChunkType splitChunkOne = assertGet(ChunkType::fromShardBSON(splitChunkOneBuilder.obj(), collVersion.epoch())); newChunks.push_back(splitChunkOne); @@ -301,7 +301,7 @@ TEST_F(ShardMetadataUtilTest, UpdateWithWriteNewChunks) { } splitChunkTwoMovedBuilder.append(ChunkType::max(), lastChunk.getMax()); splitChunkTwoMovedBuilder.append(ChunkType::shard(), "altShard"); - collVersion.appendForChunk(&splitChunkTwoMovedBuilder); + collVersion.appendLegacyWithField(&splitChunkTwoMovedBuilder, ChunkType::lastmod()); ChunkType splitChunkTwoMoved = assertGet(ChunkType::fromShardBSON(splitChunkTwoMovedBuilder.obj(), collVersion.epoch())); newChunks.push_back(splitChunkTwoMoved); diff --git a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp index c07cc18b8b5..38b6d97da1b 100644 --- a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp +++ b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp @@ -1141,7 +1141,7 @@ void ShardServerCatalogCacheLoader::CollAndChunkTaskList::addTask(collAndChunkTa } if (task.dropped) { - invariant(_tasks.back().maxQueryVersion.equals(task.minQueryVersion)); + invariant(_tasks.back().maxQueryVersion == task.minQueryVersion); // As an optimization, on collection drop, clear any pending tasks in order to prevent any // throw-away work from executing. Because we have no way to differentiate whether the @@ -1155,7 +1155,7 @@ void ShardServerCatalogCacheLoader::CollAndChunkTaskList::addTask(collAndChunkTa } } else { // Tasks must have contiguous versions, unless a complete reload occurs. - invariant(_tasks.back().maxQueryVersion.equals(task.minQueryVersion) || + invariant(_tasks.back().maxQueryVersion == task.minQueryVersion || !task.minQueryVersion.isSet()); _tasks.emplace_back(std::move(task)); diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 6cc5081bd07..963bd063fe7 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -171,8 +171,8 @@ void generateLegacyQueryErrorResponse(const AssertionException* exception, if (scex) { err.append("ok", 0.0); err.append("ns", scex->getNss().ns()); - scex->getVersionReceived().addToBSON(err, "vReceived"); - scex->getVersionWanted().addToBSON(err, "vWanted"); + scex->getVersionReceived().appendLegacyWithField(&err, "vReceived"); + scex->getVersionWanted().appendLegacyWithField(&err, "vWanted"); } BSONObj errObj = err.done(); diff --git a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp index 4bedd97b0a0..b231934c1c0 100644 --- a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp +++ b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp @@ -779,7 +779,7 @@ Status ShardingCatalogClientImpl::applyChunkOpsDeprecated(OperationContext* opCt // Look for the chunk in this shard whose version got bumped. We assume that if that // mod made it to the config server, then transaction was successful. BSONObjBuilder query; - lastChunkVersion.addToBSON(query, ChunkType::lastmod()); + lastChunkVersion.appendLegacyWithField(&query, ChunkType::lastmod()); query.append(ChunkType::ns(), nss.ns()); auto chunkWithStatus = getChunks(opCtx, query.obj(), BSONObj(), 1, nullptr, readConcern); diff --git a/src/mongo/s/catalog/type_chunk.cpp b/src/mongo/s/catalog/type_chunk.cpp index 57c610e23ad..44abf0b6c13 100644 --- a/src/mongo/s/catalog/type_chunk.cpp +++ b/src/mongo/s/catalog/type_chunk.cpp @@ -223,7 +223,7 @@ StatusWith<ChunkType> ChunkType::fromConfigBSON(const BSONObj& source) { } { - auto versionStatus = ChunkVersion::parseFromBSONForChunk(source); + auto versionStatus = ChunkVersion::parseLegacyWithField(source, ChunkType::lastmod()); if (!versionStatus.isOK()) { return versionStatus.getStatus(); } @@ -262,7 +262,7 @@ BSONObj ChunkType::toConfigBSON() const { if (_shard) builder.append(shard.name(), getShard().toString()); if (_version) - _version->appendForChunk(&builder); + _version->appendLegacyWithField(&builder, ChunkType::lastmod()); if (_jumbo) builder.append(jumbo.name(), getJumbo()); addHistoryToBSON(builder); @@ -305,11 +305,12 @@ StatusWith<ChunkType> ChunkType::fromShardBSON(const BSONObj& source, const OID& { auto statusWithChunkVersion = - ChunkVersion::parseFromBSONWithFieldAndSetEpoch(source, lastmod.name(), epoch); + ChunkVersion::parseLegacyWithField(source, ChunkType::lastmod()); if (!statusWithChunkVersion.isOK()) { return statusWithChunkVersion.getStatus(); } - chunk._version = std::move(statusWithChunkVersion.getValue()); + auto version = std::move(statusWithChunkVersion.getValue()); + chunk._version = ChunkVersion(version.majorVersion(), version.minorVersion(), epoch); } { diff --git a/src/mongo/s/catalog/type_chunk_test.cpp b/src/mongo/s/catalog/type_chunk_test.cpp index 8aa5cd1b20f..6138ceac277 100644 --- a/src/mongo/s/catalog/type_chunk_test.cpp +++ b/src/mongo/s/catalog/type_chunk_test.cpp @@ -114,7 +114,7 @@ TEST(ChunkType, MissingShardRequiredFields) { BSONObj objModLastmod = BSON( ChunkType::minShardID(kMin) << ChunkType::max(kMax) << ChunkType::shard(kShard.toString())); chunkRes = ChunkType::fromShardBSON(objModLastmod, epoch); - ASSERT_EQUALS(chunkRes.getStatus(), ErrorCodes::BadValue); + ASSERT_EQUALS(chunkRes.getStatus(), ErrorCodes::NoSuchKey); } TEST(ChunkType, ToFromShardBSON) { diff --git a/src/mongo/s/catalog/type_shard_collection.cpp b/src/mongo/s/catalog/type_shard_collection.cpp index 41c9090e570..516fa35da05 100644 --- a/src/mongo/s/catalog/type_shard_collection.cpp +++ b/src/mongo/s/catalog/type_shard_collection.cpp @@ -163,13 +163,13 @@ StatusWith<ShardCollectionType> ShardCollectionType::fromBSON(const BSONObj& sou { if (!source[lastRefreshedCollectionVersion.name()].eoo()) { auto statusWithLastRefreshedCollectionVersion = - ChunkVersion::parseFromBSONWithFieldAndSetEpoch( - source, lastRefreshedCollectionVersion.name(), epoch); + ChunkVersion::parseLegacyWithField(source, lastRefreshedCollectionVersion()); if (!statusWithLastRefreshedCollectionVersion.isOK()) { return statusWithLastRefreshedCollectionVersion.getStatus(); } + auto version = std::move(statusWithLastRefreshedCollectionVersion.getValue()); shardCollectionType.setLastRefreshedCollectionVersion( - std::move(statusWithLastRefreshedCollectionVersion.getValue())); + ChunkVersion(version.majorVersion(), version.minorVersion(), epoch)); } } diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp index 48d5177b023..134d1ed1bb5 100644 --- a/src/mongo/s/chunk_manager.cpp +++ b/src/mongo/s/chunk_manager.cpp @@ -366,7 +366,7 @@ bool RoutingTableHistory::compatibleWith(const RoutingTableHistory& other, const ShardId& shardName) const { // Return true if the shard version is the same in the two chunk managers // TODO: This doesn't need to be so strong, just major vs - return other.getVersion(shardName).equals(getVersion(shardName)); + return other.getVersion(shardName) == getVersion(shardName); } ChunkVersion RoutingTableHistory::getVersion(const ShardId& shardName) const { diff --git a/src/mongo/s/chunk_version.cpp b/src/mongo/s/chunk_version.cpp index 3b383f53e44..71879f1fdbc 100644 --- a/src/mongo/s/chunk_version.cpp +++ b/src/mongo/s/chunk_version.cpp @@ -30,42 +30,26 @@ #include "mongo/s/chunk_version.h" -#include "mongo/base/status_with.h" -#include "mongo/bson/bsonobj.h" -#include "mongo/bson/bsonobjbuilder.h" -#include "mongo/bson/util/bson_extract.h" #include "mongo/util/mongoutils/str.h" namespace mongo { -namespace { -const char kVersion[] = "version"; -const char kLastmod[] = "lastmod"; +constexpr StringData ChunkVersion::kShardVersionField; -} // namespace +StatusWith<ChunkVersion> ChunkVersion::parseWithField(const BSONObj& obj, StringData field) { + BSONElement versionElem = obj[field]; + if (versionElem.eoo()) + return {ErrorCodes::NoSuchKey, + str::stream() << "Expected field " << field << " not found."}; -const char ChunkVersion::kShardVersionField[] = "shardVersion"; - -StatusWith<ChunkVersion> ChunkVersion::parseFromBSONForCommands(const BSONObj& obj) { - return parseFromBSONWithFieldForCommands(obj, kShardVersionField); -} - -StatusWith<ChunkVersion> ChunkVersion::parseFromBSONWithFieldForCommands(const BSONObj& obj, - StringData field) { - BSONElement versionElem; - Status status = bsonExtractField(obj, field, &versionElem); - if (!status.isOK()) - return status; - - if (versionElem.type() != Array) { + if (versionElem.type() != Array) return {ErrorCodes::TypeMismatch, str::stream() << "Invalid type " << versionElem.type() << " for shardVersion element. Expected an array"}; - } BSONObjIterator it(versionElem.Obj()); if (!it.more()) - return {ErrorCodes::BadValue, "Unexpected empty version"}; + return {ErrorCodes::BadValue, "Unexpected empty version array"}; ChunkVersion version; @@ -94,49 +78,45 @@ StatusWith<ChunkVersion> ChunkVersion::parseFromBSONWithFieldForCommands(const B return version; } -StatusWith<ChunkVersion> ChunkVersion::parseFromBSONForSetShardVersion(const BSONObj& obj) { - bool canParse; - const ChunkVersion chunkVersion = ChunkVersion::fromBSON(obj, kVersion, &canParse); - if (!canParse) - return {ErrorCodes::BadValue, "Unable to parse shard version"}; - - return chunkVersion; -} - -StatusWith<ChunkVersion> ChunkVersion::parseFromBSONForChunk(const BSONObj& obj) { - bool canParse; - const ChunkVersion chunkVersion = ChunkVersion::fromBSON(obj, kLastmod, &canParse); - if (!canParse) - return {ErrorCodes::BadValue, "Unable to parse shard version"}; +StatusWith<ChunkVersion> ChunkVersion::parseLegacyWithField(const BSONObj& obj, StringData field) { + // Expect the timestamp + auto versionElem = obj[field]; + if (versionElem.eoo()) + return {ErrorCodes::NoSuchKey, + str::stream() << "Expected field " << field << " not found."}; - return chunkVersion; -} + ChunkVersion version; -StatusWith<ChunkVersion> ChunkVersion::parseFromBSONWithFieldAndSetEpoch(const BSONObj& obj, - StringData field, - const OID& epoch) { - bool canParse; - ChunkVersion chunkVersion = ChunkVersion::fromBSON(obj, field.toString(), &canParse); - if (!canParse) - return {ErrorCodes::BadValue, "Unable to parse shard version"}; - chunkVersion._epoch = epoch; - return chunkVersion; -} + if (versionElem.type() == bsonTimestamp || versionElem.type() == Date) { + version._combined = versionElem._numberLong(); + } else { + return {ErrorCodes::TypeMismatch, + str::stream() << "Invalid type " << versionElem.type() + << " for version timestamp part."}; + } -void ChunkVersion::appendForSetShardVersion(BSONObjBuilder* builder) const { - addToBSON(*builder, kVersion); -} + // Expect the optional epoch OID + const auto epochField = field + "Epoch"; + auto epochElem = obj[epochField]; + if (epochElem.type() == jstOID) { + version._epoch = epochElem.OID(); + } else if (!epochElem.eoo()) { + return {ErrorCodes::TypeMismatch, + str::stream() << "Invalid type " << epochElem.type() << " for version epoch part."}; + } -void ChunkVersion::appendForCommands(BSONObjBuilder* builder) const { - appendWithFieldForCommands(builder, kShardVersionField); + return version; } -void ChunkVersion::appendWithFieldForCommands(BSONObjBuilder* builder, StringData field) const { - builder->appendArray(field, toBSON()); +void ChunkVersion::appendWithField(BSONObjBuilder* out, StringData field) const { + BSONArrayBuilder arr(out->subarrayStart(field)); + arr.appendTimestamp(_combined); + arr.append(_epoch); } -void ChunkVersion::appendForChunk(BSONObjBuilder* builder) const { - addToBSON(*builder, kLastmod); +void ChunkVersion::appendLegacyWithField(BSONObjBuilder* out, StringData field) const { + out->appendTimestamp(field, _combined); + out->append(field + "Epoch", _epoch); } BSONObj ChunkVersion::toBSON() const { @@ -146,4 +126,8 @@ BSONObj ChunkVersion::toBSON() const { return b.arr(); } +std::string ChunkVersion::toString() const { + return str::stream() << majorVersion() << "|" << minorVersion() << "||" << _epoch; +} + } // namespace mongo diff --git a/src/mongo/s/chunk_version.h b/src/mongo/s/chunk_version.h index cbc512c72ed..6a045b0b515 100644 --- a/src/mongo/s/chunk_version.h +++ b/src/mongo/s/chunk_version.h @@ -28,14 +28,11 @@ #pragma once +#include "mongo/base/status_with.h" #include "mongo/db/jsobj.h" namespace mongo { -class BSONObj; -template <typename T> -class StatusWith; - /** * ChunkVersions consist of a major/minor version scoped to a version epoch * @@ -55,7 +52,7 @@ public: * The name for the shard version information field, which shard-aware commands should include * if they want to convey shard version. */ - static const char kShardVersionField[]; + static constexpr StringData kShardVersionField = "shardVersion"_sd; ChunkVersion() : _combined(0), _epoch(OID()) {} @@ -63,49 +60,25 @@ public: : _combined(static_cast<uint64_t>(minor) | (static_cast<uint64_t>(major) << 32)), _epoch(epoch) {} - /** - * Interprets the specified BSON content as the format for commands, which is in the form: - * { ..., shardVersion: [ <combined major/minor>, <OID epoch> ], ... } - */ - static StatusWith<ChunkVersion> parseFromBSONForCommands(const BSONObj& obj); - - /** - * Parses the BSON formatted by ChunkVersion::appendWithFieldForCommands. - * - * Interprets the specified BSON content as the format for commands, which is in the form: - * { ..., <field>: [ <combined major/minor>, <OID epoch> ], ... }. - */ - static StatusWith<ChunkVersion> parseFromBSONWithFieldForCommands(const BSONObj& obj, - StringData field); + static StatusWith<ChunkVersion> parseFromCommand(const BSONObj& obj) { + return parseWithField(obj, kShardVersionField); + } /** - * Note: if possible, use ChunkVersion::parseFromBSONForCommands or - * ChunkVersion::parseFromBSONWithFieldForCommands instead. Phasing out this function. - * - * Interprets the specified BSON content as the format for the setShardVersion command, which - * is in the form: - * { ..., version: [ <combined major/minor> ], versionEpoch: [ <OID epoch> ], ... } + * Parses the BSON formatted by appendWithField. If the field is missing, returns 'NoSuchKey', + * otherwise if the field is not properly formatted can return any relevant parsing error + * (BadValue, TypeMismatch, etc). */ - static StatusWith<ChunkVersion> parseFromBSONForSetShardVersion(const BSONObj& obj); + static StatusWith<ChunkVersion> parseWithField(const BSONObj& obj, StringData field); /** - * Note: if possible, use ChunkVersion::parseFromBSONForCommands or - * ChunkVersion::parseFromBSONWithFieldForCommands instead. Phasing out this function. + * NOTE: This format is being phased out. Use parseWithField instead. * - * Interprets the specified BSON content as the format for chunk persistence, which is in the - * form: - * { ..., lastmod: [ <combined major/minor> ], lastmodEpoch: [ <OID epoch> ], ... } - */ - static StatusWith<ChunkVersion> parseFromBSONForChunk(const BSONObj& obj); - - /** - * Interprets the lastmod (combined major/minor) from a BSONObj without an epoch - * { ..., <field>: [ <combined major/minor> ], ... } - * and then sets the returned ChunkVersion's epoch field to 'epoch'. + * Parses the BSON formatted by appendLegacyWithField. If the field is missing, returns + * 'NoSuchKey', otherwise if the field is not properly formatted can return any relevant parsing + * error (BadValue, TypeMismatch, etc). */ - static StatusWith<ChunkVersion> parseFromBSONWithFieldAndSetEpoch(const BSONObj& obj, - StringData field, - const OID& epoch); + static StatusWith<ChunkVersion> parseLegacyWithField(const BSONObj& obj, StringData field); /** * Indicates a dropped collection. All components are zeroes (OID is zero time, zero @@ -122,18 +95,15 @@ public: return ChunkVersion(0, 0, OID()); } + /** + * Indicates that the shard version checking must be skipped. + */ static ChunkVersion IGNORED() { ChunkVersion version = ChunkVersion(); version._epoch.init(Date_t(), true); // ignored OID is zero time, max machineId/inc return version; } - static ChunkVersion fromDeprecatedLong(unsigned long long num, const OID& epoch) { - ChunkVersion version(0, 0, epoch); - version._combined = num; - return version; - } - static bool isIgnoredVersion(const ChunkVersion& version) { return version.majorVersion() == 0 && version.minorVersion() == 0 && version.epoch() == IGNORED().epoch(); @@ -165,7 +135,7 @@ public: return _combined & 0xFFFF; } - OID epoch() const { + const OID& epoch() const { return _epoch; } @@ -176,7 +146,11 @@ public: // bool operator==(const ChunkVersion& otherVersion) const { - return equals(otherVersion); + return otherVersion.epoch() == epoch() && otherVersion._combined == _combined; + } + + bool operator!=(const ChunkVersion& otherVersion) const { + return !(otherVersion == *this); } bool operator>(const ChunkVersion& otherVersion) const { @@ -191,36 +165,9 @@ public: return this->_combined < otherVersion._combined; } - bool operator<=(const ChunkVersion& otherVersion) const { - return this->_combined <= otherVersion._combined; - } - - // - // Equivalence comparison types. - // - // Can we write to this data and not have a problem? - bool isWriteCompatibleWith(const ChunkVersion& otherVersion) const { - if (!hasEqualEpoch(otherVersion)) - return false; - return otherVersion.majorVersion() == majorVersion(); - } - - // Is this the same version? - bool equals(const ChunkVersion& otherVersion) const { - if (!hasEqualEpoch(otherVersion)) - return false; - return otherVersion._combined == _combined; - } - - /** - * Returns true if the otherVersion is the same as this version and enforces strict epoch - * checking (empty epochs are not wildcards). - */ - bool isStrictlyEqualTo(const ChunkVersion& otherVersion) const { - if (otherVersion._epoch != _epoch) - return false; - return otherVersion._combined == _combined; + bool isWriteCompatibleWith(const ChunkVersion& other) const { + return epoch() == other.epoch() && majorVersion() == other.majorVersion(); } /** @@ -238,175 +185,29 @@ public: return minorVersion() < otherVersion.minorVersion(); } - // Is this in the same epoch? - bool hasEqualEpoch(const ChunkVersion& otherVersion) const { - return hasEqualEpoch(otherVersion._epoch); + void appendToCommand(BSONObjBuilder* out) const { + appendWithField(out, kShardVersionField); } - bool hasEqualEpoch(const OID& otherEpoch) const { - return _epoch == otherEpoch; - } - - // - // BSON input/output - // - // The idea here is to make the BSON input style very flexible right now, so we - // can then tighten it up in the next version. We can accept either a BSONObject field - // with version and epoch, or version and epoch in different fields (either is optional). - // In this case, epoch always is stored in a field name of the version field name + "Epoch" - // - - // - // { version : <TS> } and { version : [<TS>,<OID>] } format - // - - static ChunkVersion fromBSON(const BSONElement& el, const std::string& prefix, bool* canParse) { - *canParse = true; - - int type = el.type(); - - if (type == Array) { - return fromBSON(BSONArray(el.Obj()), canParse); - } - - if (type == jstOID) { - return ChunkVersion(0, 0, el.OID()); - } - - if (type == bsonTimestamp || type == Date) { - return fromDeprecatedLong(el._numberLong(), OID()); - } - - *canParse = false; - - return ChunkVersion(0, 0, OID()); - } - - // - // { version : <TS>, versionEpoch : <OID> } object format - // - - static ChunkVersion fromBSON(const BSONObj& obj, const std::string& prefix = "") { - bool canParse; - return fromBSON(obj, prefix, &canParse); - } - - static ChunkVersion fromBSON(const BSONObj& obj, const std::string& prefixIn, bool* canParse) { - *canParse = true; - - std::string prefix = prefixIn; - // "version" doesn't have a "cluster constanst" because that field is never - // written to the config. - if (prefixIn == "" && !obj["version"].eoo()) { - prefix = (std::string) "version"; - } - // TODO: use ChunkType::lastmod() - // NOTE: type_chunk.h includes this file - else if (prefixIn == "" && !obj["lastmod"].eoo()) { - prefix = (std::string) "lastmod"; - } - - ChunkVersion version = fromBSON(obj[prefix], prefixIn, canParse); - - if (obj[prefix + "Epoch"].type() == jstOID) { - version._epoch = obj[prefix + "Epoch"].OID(); - *canParse = true; - } - - return version; - } - - // - // { version : [<TS>, <OID>] } format - // - - static ChunkVersion fromBSON(const BSONArray& arr, bool* canParse) { - *canParse = false; - - ChunkVersion version; - - BSONObjIterator it(arr); - if (!it.more()) - return version; - - version = fromBSON(it.next(), "", canParse); - if (!(*canParse)) - return version; - - *canParse = true; - - if (!it.more()) - return version; - BSONElement next = it.next(); - if (next.type() != jstOID) - return version; - - version._epoch = next.OID(); - - return version; - } - - // - // Currently our BSON output is to two different fields, to cleanly work with older - // versions that know nothing about epochs. - // - - BSONObj toBSONWithPrefix(const std::string& prefix) const { - invariant(!prefix.empty()); - - BSONObjBuilder b; - b.appendTimestamp(prefix, _combined); - b.append(prefix + "Epoch", _epoch); - return b.obj(); - } - - /** - * Note: if possible, use ChunkVersion::appendForCommands or - * ChunkVersion::appendWithFieldForCommands instead. Phasing out this function. - */ - void addToBSON(BSONObjBuilder& b, const std::string& prefix) const { - b.appendElements(toBSONWithPrefix(prefix)); - } - - /** - * Appends the contents to the specified builder in the format expected by the setShardVersion - * command. - */ - void appendForSetShardVersion(BSONObjBuilder* builder) const; - /** - * Appends the contents to the specified builder in the format expected by the sharded commands. + * Serializes the version held by this object to 'out' in the form: + * { ..., <field>: [ <combined major/minor>, <OID epoch> ], ... }. */ - void appendForCommands(BSONObjBuilder* builder) const; + void appendWithField(BSONObjBuilder* out, StringData field) const; /** - * Appends the contents as an array to "builder" with the field name "field" in the format - * expected by the sharded commands. - * - * { ..., <field>: [ <combined major/minor>, <OID epoch> ], ... } + * NOTE: This format is being phased out. Use appendWithField instead. * - * Use ChunkVersion::parseFromBSONWithFieldForCommands to retrieve the ChunkVersion from the - * BSON created by this function. + * Serializes the version held by this object to 'out' in the legacy form: + * { ..., <field>: [ <combined major/minor> ], <field>Epoch: [ <OID epoch> ], ... } */ - void appendWithFieldForCommands(BSONObjBuilder* builder, StringData field) const; - - /** - * Appends the contents to the specified builder in the format expected by the chunk - * serialization/deserialization code. - */ - void appendForChunk(BSONObjBuilder* builder) const; - - std::string toString() const { - StringBuilder sb; - sb << majorVersion() << "|" << minorVersion() << "||" << _epoch; - return sb.str(); - } + void appendLegacyWithField(BSONObjBuilder* out, StringData field) const; BSONObj toBSON() const; + std::string toString() const; private: uint64_t _combined; - OID _epoch; }; diff --git a/src/mongo/s/chunk_version_test.cpp b/src/mongo/s/chunk_version_test.cpp index 4bea7f466bd..38512c0df38 100644 --- a/src/mongo/s/chunk_version_test.cpp +++ b/src/mongo/s/chunk_version_test.cpp @@ -28,45 +28,100 @@ #include "mongo/platform/basic.h" -#include "mongo/db/jsobj.h" #include "mongo/s/chunk_version.h" #include "mongo/unittest/unittest.h" namespace mongo { namespace { -TEST(Parsing, EpochIsOptional) { +using unittest::assertGet; + +TEST(ChunkVersionParsing, ToFromBSONRoundtrip) { + ChunkVersion version(1, 2, OID::gen()); + const auto roundTripVersion = assertGet(ChunkVersion::parseWithField( + [&] { + BSONObjBuilder builder; + version.appendWithField(&builder, "testVersionField"); + return builder.obj(); + }(), + "testVersionField")); + + ASSERT_EQ(version, roundTripVersion); +} + +TEST(ChunkVersionParsing, ToFromBSONLegacyRoundtrip) { + ChunkVersion version(1, 2, OID::gen()); + const auto roundTripVersion = assertGet(ChunkVersion::parseLegacyWithField( + [&] { + BSONObjBuilder builder; + version.appendLegacyWithField(&builder, "testVersionField"); + return builder.obj(); + }(), + "testVersionField")); + + ASSERT_EQ(version, roundTripVersion); +} + +TEST(ChunkVersionParsing, FromBSON) { + const OID oid = OID::gen(); + ChunkVersion chunkVersionComplete = assertGet(ChunkVersion::parseWithField( + BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3) << oid)), + "testVersionField")); + + ASSERT(chunkVersionComplete.epoch().isSet()); + ASSERT_EQ(oid, chunkVersionComplete.epoch()); + ASSERT_EQ(2, chunkVersionComplete.majorVersion()); + ASSERT_EQ(3, chunkVersionComplete.minorVersion()); +} + +TEST(ChunkVersionParsing, FromBSONMissingEpoch) { + ASSERT_THROWS_CODE( + uassertStatusOK(ChunkVersion::parseWithField( + BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3))), "testVersionField")), + AssertionException, + ErrorCodes::TypeMismatch); +} + +TEST(ChunkVersionParsing, FromBSONMissingTimestamp) { const OID oid = OID::gen(); - bool canParse = false; + ASSERT_THROWS_CODE(uassertStatusOK(ChunkVersion::parseWithField(BSON("testVersionField" << oid), + "testVersionField")), + AssertionException, + ErrorCodes::TypeMismatch); +} + +TEST(ChunkVersionParsing, FromBSONLegacy) { + const OID oid = OID::gen(); + ChunkVersion chunkVersionComplete = assertGet(ChunkVersion::parseLegacyWithField( + BSON("lastmod" << Timestamp(Seconds(2), 3) << "lastmodEpoch" << oid), "lastmod")); - ChunkVersion chunkVersionComplete = ChunkVersion::fromBSON( - BSON("lastmod" << Timestamp(Seconds(2), 3) << "lastmodEpoch" << oid), "lastmod", &canParse); - ASSERT(canParse); ASSERT(chunkVersionComplete.epoch().isSet()); - ASSERT(chunkVersionComplete.epoch() == oid); + ASSERT_EQ(oid, chunkVersionComplete.epoch()); ASSERT_EQ(2, chunkVersionComplete.majorVersion()); ASSERT_EQ(3, chunkVersionComplete.minorVersion()); +} + +TEST(ChunkVersionParsing, FromBSONLegacyEpochIsOptional) { + ChunkVersion chunkVersionNoEpoch = assertGet( + ChunkVersion::parseLegacyWithField(BSON("lastmod" << Timestamp(Seconds(3), 4)), "lastmod")); - canParse = false; - ChunkVersion chunkVersionNoEpoch = - ChunkVersion::fromBSON(BSON("lastmod" << Timestamp(Seconds(3), 4)), "lastmod", &canParse); - ASSERT(canParse); ASSERT(!chunkVersionNoEpoch.epoch().isSet()); ASSERT_EQ(3, chunkVersionNoEpoch.majorVersion()); ASSERT_EQ(4, chunkVersionNoEpoch.minorVersion()); } -TEST(Comparison, StrictEqual) { +TEST(ChunkVersionComparison, EqualityOperators) { OID epoch = OID::gen(); - ASSERT(ChunkVersion(3, 1, epoch).isStrictlyEqualTo(ChunkVersion(3, 1, epoch))); - ASSERT(!ChunkVersion(3, 1, epoch).isStrictlyEqualTo(ChunkVersion(3, 1, OID()))); - ASSERT(!ChunkVersion(3, 1, OID()).isStrictlyEqualTo(ChunkVersion(3, 1, epoch))); - ASSERT(ChunkVersion(3, 1, OID()).isStrictlyEqualTo(ChunkVersion(3, 1, OID()))); - ASSERT(!ChunkVersion(4, 2, epoch).isStrictlyEqualTo(ChunkVersion(4, 1, epoch))); + ASSERT_EQ(ChunkVersion(3, 1, epoch), ChunkVersion(3, 1, epoch)); + ASSERT_EQ(ChunkVersion(3, 1, OID()), ChunkVersion(3, 1, OID())); + + ASSERT_NE(ChunkVersion(3, 1, epoch), ChunkVersion(3, 1, OID())); + ASSERT_NE(ChunkVersion(3, 1, OID()), ChunkVersion(3, 1, epoch)); + ASSERT_NE(ChunkVersion(4, 2, epoch), ChunkVersion(4, 1, epoch)); } -TEST(Comparison, OlderThan) { +TEST(ChunkVersionComparison, OlderThan) { OID epoch = OID::gen(); ASSERT(ChunkVersion(3, 1, epoch).isOlderThan(ChunkVersion(4, 1, epoch))); @@ -83,5 +138,5 @@ TEST(Comparison, OlderThan) { ASSERT(!ChunkVersion(3, 1, epoch).isOlderThan(ChunkVersion(3, 1, epoch))); } -} // unnamed namespace +} // namespace } // namespace mongo diff --git a/src/mongo/s/commands/cluster_commands_helpers.cpp b/src/mongo/s/commands/cluster_commands_helpers.cpp index bf5c4351c6e..2820cc62d91 100644 --- a/src/mongo/s/commands/cluster_commands_helpers.cpp +++ b/src/mongo/s/commands/cluster_commands_helpers.cpp @@ -236,7 +236,7 @@ std::vector<AsyncRequestsSender::Response> gatherResponses( BSONObj appendShardVersion(BSONObj cmdObj, ChunkVersion version) { BSONObjBuilder cmdWithVersionBob(std::move(cmdObj)); - version.appendForCommands(&cmdWithVersionBob); + version.appendToCommand(&cmdWithVersionBob); return cmdWithVersionBob.obj(); } diff --git a/src/mongo/s/commands/cluster_get_shard_version_cmd.cpp b/src/mongo/s/commands/cluster_get_shard_version_cmd.cpp index 85afe515116..f2bd4975fc8 100644 --- a/src/mongo/s/commands/cluster_get_shard_version_cmd.cpp +++ b/src/mongo/s/commands/cluster_get_shard_version_cmd.cpp @@ -112,7 +112,7 @@ public: log() << redact(chunk.toString()); } - cm->getVersion().addToBSON(result, "version"); + cm->getVersion().appendLegacyWithField(&result, "version"); } return true; diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp index 2ff23072251..0cc9cc3a336 100644 --- a/src/mongo/s/query/cluster_find.cpp +++ b/src/mongo/s/query/cluster_find.cpp @@ -198,11 +198,9 @@ std::vector<std::pair<ShardId, BSONObj>> constructRequestsForShards( qrToForward->asFindCommand(&cmdBuilder); if (routingInfo.cm()) { - ChunkVersion version(routingInfo.cm()->getVersion(shardId)); - version.appendForCommands(&cmdBuilder); + routingInfo.cm()->getVersion(shardId).appendToCommand(&cmdBuilder); } else if (!query.nss().isOnInternalDb()) { - ChunkVersion version(ChunkVersion::UNSHARDED()); - version.appendForCommands(&cmdBuilder); + ChunkVersion::UNSHARDED().appendToCommand(&cmdBuilder); } // TODO SERVER-33702: standardize method for attaching txnNumber through mongos. diff --git a/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp b/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp index 7800657e5cf..c7e946ad293 100644 --- a/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp +++ b/src/mongo/s/request_types/commit_chunk_migration_request_type.cpp @@ -112,7 +112,7 @@ StatusWith<CommitChunkMigrationRequest> CommitChunkMigrationRequest::createFromC { auto statusWithChunkVersion = - ChunkVersion::parseFromBSONWithFieldForCommands(obj, kFromShardCollectionVersion); + ChunkVersion::parseWithField(obj, kFromShardCollectionVersion); if (!statusWithChunkVersion.isOK()) { return statusWithChunkVersion.getStatus(); } @@ -151,7 +151,7 @@ void CommitChunkMigrationRequest::appendAsCommand(BSONObjBuilder* builder, builder->append(kFromShard, fromShard.toString()); builder->append(kToShard, toShard.toString()); builder->append(kMigratedChunk, migratedChunk.toConfigBSON()); - fromShardCollectionVersion.appendWithFieldForCommands(builder, kFromShardCollectionVersion); + fromShardCollectionVersion.appendWithField(builder, kFromShardCollectionVersion); builder->append(kValidAfter, validAfter); } diff --git a/src/mongo/s/request_types/move_chunk_request.cpp b/src/mongo/s/request_types/move_chunk_request.cpp index d1d5183672b..cd7b3eab4d0 100644 --- a/src/mongo/s/request_types/move_chunk_request.cpp +++ b/src/mongo/s/request_types/move_chunk_request.cpp @@ -153,7 +153,7 @@ void MoveChunkRequest::appendAsCommand(BSONObjBuilder* builder, invariant(nss.isValid()); builder->append(kMoveChunk, nss.ns()); - chunkVersion.appendForCommands(builder); // 3.4 shard compatibility + chunkVersion.appendToCommand(builder); // 3.4 shard compatibility builder->append(kEpoch, chunkVersion.epoch()); // config connection string is included for 3.4 shard compatibility builder->append(kConfigServerConnectionString, configServerConnectionString.toString()); diff --git a/src/mongo/s/request_types/set_shard_version_request.cpp b/src/mongo/s/request_types/set_shard_version_request.cpp index 68f72f80132..7a3bfbcb798 100644 --- a/src/mongo/s/request_types/set_shard_version_request.cpp +++ b/src/mongo/s/request_types/set_shard_version_request.cpp @@ -52,6 +52,8 @@ const char kNoConnectionVersioning[] = "noConnectionVersioning"; } // namespace +constexpr StringData SetShardVersionRequest::kVersion; + SetShardVersionRequest::SetShardVersionRequest(ConnectionString configServer, ShardId shardName, ConnectionString shardConnectionString) @@ -194,7 +196,7 @@ StatusWith<SetShardVersionRequest> SetShardVersionRequest::parseFromBSON(const B } { - auto versionStatus = ChunkVersion::parseFromBSONForSetShardVersion(cmdObj); + auto versionStatus = ChunkVersion::parseLegacyWithField(cmdObj, kVersion); if (!versionStatus.isOK()) return versionStatus.getStatus(); @@ -221,7 +223,7 @@ BSONObj SetShardVersionRequest::toBSON() const { // SERVER-21458. cmdBuilder.append(QueryRequest::cmdOptionMaxTimeMS, 30000); } else { - _version.get().appendForSetShardVersion(&cmdBuilder); + _version->appendLegacyWithField(&cmdBuilder, kVersion); } if (_noConnectionVersioning) { diff --git a/src/mongo/s/request_types/set_shard_version_request.h b/src/mongo/s/request_types/set_shard_version_request.h index 7ebdf9de54a..7fa594c46c1 100644 --- a/src/mongo/s/request_types/set_shard_version_request.h +++ b/src/mongo/s/request_types/set_shard_version_request.h @@ -47,6 +47,8 @@ class StatusWith; */ class SetShardVersionRequest { public: + static constexpr StringData kVersion = "version"_sd; + /** * Constructs a new set shard version request, which is of the "init" type, meaning it has no * namespace or version information associated with it and the init flag is set. diff --git a/src/mongo/s/request_types/set_shard_version_request_test.cpp b/src/mongo/s/request_types/set_shard_version_request_test.cpp index 5163b6f1779..e93f156d87c 100644 --- a/src/mongo/s/request_types/set_shard_version_request_test.cpp +++ b/src/mongo/s/request_types/set_shard_version_request_test.cpp @@ -267,8 +267,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFull) { ASSERT_EQ(ssv.getShardName(), "TestShard"); ASSERT_EQ(ssv.getShardConnectionString().toString(), shardCS.toString()); ASSERT_EQ(ssv.getNS().ns(), "db.coll"); - ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSONWithPrefix("version"), - chunkVersion.toBSONWithPrefix("version")); + ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON()); ASSERT_BSONOBJ_EQ(ssv.toBSON(), BSON("setShardVersion" @@ -304,8 +303,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullAuthoritative) { ASSERT_EQ(ssv.getShardName(), "TestShard"); ASSERT_EQ(ssv.getShardConnectionString().toString(), shardCS.toString()); ASSERT_EQ(ssv.getNS().ns(), "db.coll"); - ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSONWithPrefix("version"), - chunkVersion.toBSONWithPrefix("version")); + ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON()); ASSERT_BSONOBJ_EQ(ssv.toBSON(), BSON("setShardVersion" @@ -347,8 +345,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullForceRefresh) { ASSERT_EQ(ssv.getShardName(), "TestShard"); ASSERT_EQ(ssv.getShardConnectionString().toString(), shardCS.toString()); ASSERT_EQ(ssv.getNS().ns(), "db.coll"); - ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSONWithPrefix("version"), - chunkVersion.toBSONWithPrefix("version")); + ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON()); ASSERT_BSONOBJ_EQ(ssv.toBSON(), BSON("setShardVersion" @@ -384,8 +381,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullNoConnectionVersioning) { ASSERT_EQ(ssv.getShardName(), "TestShard"); ASSERT_EQ(ssv.getShardConnectionString().toString(), shardCS.toString()); ASSERT_EQ(ssv.getNS().ns(), "db.coll"); - ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSONWithPrefix("version"), - chunkVersion.toBSONWithPrefix("version")); + ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON()); ASSERT_BSONOBJ_EQ(ssv.toBSON(), BSON("setShardVersion" @@ -429,8 +425,7 @@ TEST(SetShardVersionRequest, ToSSVCommandFullNoConnectionVersioningForceRefresh) ASSERT_EQ(ssv.getShardName(), "TestShard"); ASSERT_EQ(ssv.getShardConnectionString().toString(), shardCS.toString()); ASSERT_EQ(ssv.getNS().ns(), "db.coll"); - ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSONWithPrefix("version"), - chunkVersion.toBSONWithPrefix("version")); + ASSERT_BSONOBJ_EQ(ssv.getNSVersion().toBSON(), chunkVersion.toBSON()); ASSERT_BSONOBJ_EQ(ssv.toBSON(), BSON("setShardVersion" diff --git a/src/mongo/s/shard_util.cpp b/src/mongo/s/shard_util.cpp index b43aee32537..bc7a36073d2 100644 --- a/src/mongo/s/shard_util.cpp +++ b/src/mongo/s/shard_util.cpp @@ -175,7 +175,8 @@ StatusWith<boost::optional<ChunkRange>> splitChunkAtMultiplePoints( cmd.append("from", shardId.toString()); cmd.append("keyPattern", shardKeyPattern.toBSON()); cmd.append("epoch", collectionVersion.epoch()); - collectionVersion.appendForCommands(&cmd); // backwards compatibility with v3.4 + collectionVersion.appendWithField( + &cmd, ChunkVersion::kShardVersionField); // backwards compatibility with v3.4 chunkRange.append(&cmd); cmd.append("splitKeys", splitPoints); diff --git a/src/mongo/s/stale_exception.cpp b/src/mongo/s/stale_exception.cpp index 29c1c1ad7e1..3c3f63a26ed 100644 --- a/src/mongo/s/stale_exception.cpp +++ b/src/mongo/s/stale_exception.cpp @@ -39,17 +39,24 @@ namespace { MONGO_INIT_REGISTER_ERROR_EXTRA_INFO(StaleConfigInfo); MONGO_INIT_REGISTER_ERROR_EXTRA_INFO(StaleDbRoutingVersion); +ChunkVersion extractOptionalVersion(const BSONObj& obj, StringData field) { + auto swChunkVersion = ChunkVersion::parseLegacyWithField(obj, field); + if (swChunkVersion == ErrorCodes::NoSuchKey) + return ChunkVersion::UNSHARDED(); + return uassertStatusOK(std::move(swChunkVersion)); +} + } // namespace StaleConfigInfo::StaleConfigInfo(const BSONObj& obj) : StaleConfigInfo(NamespaceString(obj["ns"].type() == String ? obj["ns"].String() : ""), - ChunkVersion::fromBSON(obj, "vReceived"), - ChunkVersion::fromBSON(obj, "vWanted")) {} + extractOptionalVersion(obj, "vReceived"), + extractOptionalVersion(obj, "vWanted")) {} void StaleConfigInfo::serialize(BSONObjBuilder* bob) const { bob->append("ns", _nss.ns()); - _received.addToBSON(*bob, "vReceived"); - _wanted.addToBSON(*bob, "vWanted"); + _received.appendLegacyWithField(bob, "vReceived"); + _wanted.appendLegacyWithField(bob, "vWanted"); } std::shared_ptr<const ErrorExtraInfo> StaleConfigInfo::parse(const BSONObj& obj) { diff --git a/src/mongo/s/write_ops/batched_command_request.cpp b/src/mongo/s/write_ops/batched_command_request.cpp index 4f1d118c3ca..c500bc201eb 100644 --- a/src/mongo/s/write_ops/batched_command_request.cpp +++ b/src/mongo/s/write_ops/batched_command_request.cpp @@ -42,7 +42,7 @@ template <class T> BatchedCommandRequest constructBatchedCommandRequest(const OpMsgRequest& request) { auto batchRequest = BatchedCommandRequest{T::parse(request)}; - auto chunkVersion = ChunkVersion::parseFromBSONForCommands(request.body); + auto chunkVersion = ChunkVersion::parseFromCommand(request.body); if (chunkVersion != ErrorCodes::NoSuchKey) { batchRequest.setShardVersion(uassertStatusOK(std::move(chunkVersion))); } @@ -136,7 +136,7 @@ void BatchedCommandRequest::setWriteCommandBase(write_ops::WriteCommandBase writ void BatchedCommandRequest::serialize(BSONObjBuilder* builder) const { _visit([&](auto&& op) { op.serialize({}, builder); }); if (_shardVersion) { - _shardVersion->appendForCommands(builder); + _shardVersion->appendToCommand(builder); } if (_writeConcern) { diff --git a/src/mongo/s/write_ops/chunk_manager_targeter.cpp b/src/mongo/s/write_ops/chunk_manager_targeter.cpp index 9375405adae..a856c3db9d5 100644 --- a/src/mongo/s/write_ops/chunk_manager_targeter.cpp +++ b/src/mongo/s/write_ops/chunk_manager_targeter.cpp @@ -200,7 +200,7 @@ bool isExactIdQuery(OperationContext* opCtx, CompareResult compareShardVersions(const ChunkVersion& shardVersionA, const ChunkVersion& shardVersionB) { // Collection may have been dropped - if (!shardVersionA.hasEqualEpoch(shardVersionB)) { + if (shardVersionA.epoch() != shardVersionB.epoch()) { return CompareResult_Unknown; } @@ -288,10 +288,9 @@ bool isMetadataDifferent(const std::shared_ptr<ChunkManager>& managerA, return true; if (managerA) { - return !managerA->getVersion().isStrictlyEqualTo(managerB->getVersion()); + return managerA->getVersion() != managerB->getVersion(); } - dassert(NULL != primaryA.get()); return primaryA->getId() != primaryB->getId(); } @@ -699,7 +698,8 @@ void ChunkManagerTargeter::noteStaleResponse(const ShardEndpoint& endpoint, remoteShardVersion = getShardVersion(*_routingInfo, endpoint.shardName); remoteShardVersion.incMajor(); } else { - remoteShardVersion = ChunkVersion::fromBSON(staleInfo, "vWanted"); + remoteShardVersion = + uassertStatusOK(ChunkVersion::parseLegacyWithField(staleInfo, "vWanted")); } ShardVersionMap::iterator it = _remoteShardVersions.find(endpoint.shardName); @@ -707,7 +707,7 @@ void ChunkManagerTargeter::noteStaleResponse(const ShardEndpoint& endpoint, _remoteShardVersions.insert(std::make_pair(endpoint.shardName, remoteShardVersion)); } else { ChunkVersion& previouslyNotedVersion = it->second; - if (previouslyNotedVersion.hasEqualEpoch(remoteShardVersion)) { + if (previouslyNotedVersion.epoch() == remoteShardVersion.epoch()) { if (previouslyNotedVersion.isOlderThan(remoteShardVersion)) { previouslyNotedVersion = remoteShardVersion; } |