diff options
author | Kaloian Manassiev <kaloian.manassiev@mongodb.com> | 2022-01-24 14:56:18 +0100 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-01-24 20:03:34 +0000 |
commit | a2aff33b1f4fa49efd85b245e7945e720a775530 (patch) | |
tree | 8dc283bae55d6b8ac2e797ce328d5244eacdd8bd /src | |
parent | f395e9d34b413781bcfb67c2674378734223466c (diff) | |
download | mongo-a2aff33b1f4fa49efd85b245e7945e720a775530.tar.gz |
SERVER-62783 Get rid of ChunkVersion::appendWithField/parseWithField
Diffstat (limited to 'src')
27 files changed, 98 insertions, 137 deletions
diff --git a/src/mongo/db/query/query_request_helper.h b/src/mongo/db/query/query_request_helper.h index 4044083d89f..4d3ec6143c8 100644 --- a/src/mongo/db/query/query_request_helper.h +++ b/src/mongo/db/query/query_request_helper.h @@ -57,8 +57,6 @@ static constexpr auto kMaxTimeMSOpOnlyField = "maxTimeMSOpOnly"; // Field names for sorting options. static constexpr auto kNaturalSortField = "$natural"; -static constexpr auto kShardVersionField = "shardVersion"; - /** * Assert that collectionName is valid. */ diff --git a/src/mongo/db/s/balancer/balancer_commands_scheduler_impl.h b/src/mongo/db/s/balancer/balancer_commands_scheduler_impl.h index e8bc271bf9b..e81bbc97a83 100644 --- a/src/mongo/db/s/balancer/balancer_commands_scheduler_impl.h +++ b/src/mongo/db/s/balancer/balancer_commands_scheduler_impl.h @@ -239,7 +239,7 @@ public: .append(kShardName, getTarget().toString()) .append(kEpoch, _version.epoch()); - _version.appendWithField(&commandBuilder, ChunkVersion::kShardVersionField); + _version.serializeToBSON(ChunkVersion::kShardVersionField, &commandBuilder); return commandBuilder.obj(); } @@ -314,7 +314,7 @@ public: .append(kMaxValue, _upperBoundKey) .append(kEstimatedValue, _estimatedValue); - _version.appendWithField(&commandBuilder, ChunkVersion::kShardVersionField); + _version.serializeToBSON(ChunkVersion::kShardVersionField, &commandBuilder); return commandBuilder.obj(); } diff --git a/src/mongo/db/s/balancer/migration_test_fixture.cpp b/src/mongo/db/s/balancer/migration_test_fixture.cpp index b970710cbe2..59137d19e7f 100644 --- a/src/mongo/db/s/balancer/migration_test_fixture.cpp +++ b/src/mongo/db/s/balancer/migration_test_fixture.cpp @@ -125,7 +125,7 @@ void MigrationTestFixture::setUpMigration(const NamespaceString& ns, builder.append(MigrationType::max(), chunk.getMax()); builder.append(MigrationType::toShard(), toShard.toString()); builder.append(MigrationType::fromShard(), chunk.getShard().toString()); - chunk.getVersion().appendWithField(&builder, "chunkVersion"); + chunk.getVersion().serializeToBSON("chunkVersion", &builder); builder.append(MigrationType::forceJumbo(), "doNotForceJumbo"); MigrationType migrationType = assertGet(MigrationType::fromBSON(builder.obj())); diff --git a/src/mongo/db/s/balancer/type_migration.cpp b/src/mongo/db/s/balancer/type_migration.cpp index fc1371cef4a..8be64f74323 100644 --- a/src/mongo/db/s/balancer/type_migration.cpp +++ b/src/mongo/db/s/balancer/type_migration.cpp @@ -115,11 +115,11 @@ StatusWith<MigrationType> MigrationType::fromBSON(const BSONObj& source) { migrationType._fromShard = std::move(migrationFromShard); } - { - auto chunkVersionStatus = ChunkVersion::parseWithField(source, chunkVersion.name()); - if (!chunkVersionStatus.isOK()) - return chunkVersionStatus.getStatus(); - migrationType._chunkVersion = chunkVersionStatus.getValue(); + try { + auto chunkVersionStatus = ChunkVersion::fromBSONArrayThrowing(source[chunkVersion.name()]); + migrationType._chunkVersion = chunkVersionStatus; + } catch (const DBException& ex) { + return ex.toStatus(); } { @@ -160,7 +160,7 @@ BSONObj MigrationType::toBSON() const { builder.append(fromShard.name(), _fromShard.toString()); builder.append(toShard.name(), _toShard.toString()); - _chunkVersion.appendWithField(&builder, chunkVersion.name()); + _chunkVersion.serializeToBSON(chunkVersion.name(), &builder); builder.append(waitForDelete.name(), _waitForDelete); builder.append(forceJumbo.name(), _forceJumbo); diff --git a/src/mongo/db/s/balancer/type_migration_test.cpp b/src/mongo/db/s/balancer/type_migration_test.cpp index c779efd4d44..e6bc584bca3 100644 --- a/src/mongo/db/s/balancer/type_migration_test.cpp +++ b/src/mongo/db/s/balancer/type_migration_test.cpp @@ -78,7 +78,7 @@ TEST(MigrationTypeTest, ConvertFromMigrationInfo) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); builder.append(MigrationType::waitForDelete(), kWaitForDelete); builder.append(MigrationType::forceJumbo(), MoveChunkRequest::forceJumboToString(MoveChunkRequest::ForceJumbo::kDoNotForce)); @@ -97,7 +97,7 @@ TEST(MigrationTypeTest, FromAndToBSON) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); builder.append(MigrationType::waitForDelete(), kWaitForDelete); builder.append(MigrationType::forceJumbo(), MoveChunkRequest::forceJumboToString(MoveChunkRequest::ForceJumbo::kDoNotForce)); @@ -116,7 +116,7 @@ TEST(MigrationTypeTest, MissingRequiredNamespaceField) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); BSONObj obj = builder.obj(); @@ -133,7 +133,7 @@ TEST(MigrationTypeTest, MissingRequiredMinField) { builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); BSONObj obj = builder.obj(); @@ -150,7 +150,7 @@ TEST(MigrationTypeTest, MissingRequiredMaxField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::fromShard(), kFromShard.toString()); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); BSONObj obj = builder.obj(); @@ -167,7 +167,7 @@ TEST(MigrationTypeTest, MissingRequiredFromShardField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::max(), kMax); builder.append(MigrationType::toShard(), kToShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); BSONObj obj = builder.obj(); @@ -184,7 +184,7 @@ TEST(MigrationTypeTest, MissingRequiredToShardField) { builder.append(MigrationType::min(), kMin); builder.append(MigrationType::max(), kMax); builder.append(MigrationType::fromShard(), kFromShard.toString()); - version.appendWithField(&builder, "chunkVersion"); + version.serializeToBSON("chunkVersion", &builder); BSONObj obj = builder.obj(); @@ -203,9 +203,7 @@ TEST(MigrationTypeTest, MissingRequiredVersionField) { BSONObj obj = builder.obj(); - StatusWith<MigrationType> migrationType = MigrationType::fromBSON(obj); - ASSERT_EQUALS(migrationType.getStatus(), ErrorCodes::NoSuchKey); - ASSERT_STRING_CONTAINS(migrationType.getStatus().reason(), "chunkVersion"); + ASSERT_THROWS(uassertStatusOK(MigrationType::fromBSON(obj)), DBException); } } // namespace diff --git a/src/mongo/db/s/collection_metadata_filtering_test.cpp b/src/mongo/db/s/collection_metadata_filtering_test.cpp index 8a485cd0c86..f87803ec350 100644 --- a/src/mongo/db/s/collection_metadata_filtering_test.cpp +++ b/src/mongo/db/s/collection_metadata_filtering_test.cpp @@ -127,7 +127,7 @@ protected: const auto version = cm.getVersion(ShardId("0")); BSONObjBuilder builder; - version.appendWithField(&builder, ChunkVersion::kShardVersionField); + version.serializeToBSON(ChunkVersion::kShardVersionField, &builder); auto& oss = OperationShardingState::get(operationContext()); oss.initializeClientRoutingVersionsFromCommand(kNss, builder.obj()); } diff --git a/src/mongo/db/s/collection_sharding_runtime_test.cpp b/src/mongo/db/s/collection_sharding_runtime_test.cpp index 70eaccd5105..00fdf7872bd 100644 --- a/src/mongo/db/s/collection_sharding_runtime_test.cpp +++ b/src/mongo/db/s/collection_sharding_runtime_test.cpp @@ -80,7 +80,7 @@ protected: if (!OperationShardingState::isOperationVersioned(opCtx)) { const auto version = cm.getVersion(ShardId("0")); BSONObjBuilder builder; - version.appendWithField(&builder, ChunkVersion::kShardVersionField); + version.serializeToBSON(ChunkVersion::kShardVersionField, &builder); auto& oss = OperationShardingState::get(opCtx); oss.initializeClientRoutingVersionsFromCommand(kTestNss, builder.obj()); } 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 51e0a838d87..6025201b5a8 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 @@ -695,8 +695,8 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkSplit( } BSONObjBuilder response; - currentMaxVersion.appendWithField(&response, kCollectionVersionField); - currentMaxVersion.appendWithField(&response, ChunkVersion::kShardVersionField); + currentMaxVersion.serializeToBSON(kCollectionVersionField, &response); + currentMaxVersion.serializeToBSON(ChunkVersion::kShardVersionField, &response); return response.obj(); } @@ -773,10 +773,10 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunksMerge( << chunkRange.toString(), chunk.getRange() == chunkRange); BSONObjBuilder response; - swCollVersion.getValue().appendWithField(&response, kCollectionVersionField); + swCollVersion.getValue().serializeToBSON(kCollectionVersionField, &response); const auto currentShardVersion = getShardVersion(opCtx, coll, shardId, swCollVersion.getValue()); - currentShardVersion.appendWithField(&response, ChunkVersion::kShardVersionField); + currentShardVersion.serializeToBSON(ChunkVersion::kShardVersionField, &response); // Makes sure that the last thing we read in getCollectionVersion and getShardVersion gets // majority written before to return from this command, otherwise next RoutingInfo cache // refresh from the shard may not see those newest information. @@ -848,8 +848,8 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunksMerge( opCtx, "merge", nss.ns(), logDetail.obj(), WriteConcernOptions()); BSONObjBuilder response; - mergeVersion.appendWithField(&response, kCollectionVersionField); - mergeVersion.appendWithField(&response, ChunkVersion::kShardVersionField); + mergeVersion.serializeToBSON(kCollectionVersionField, &response); + mergeVersion.serializeToBSON(ChunkVersion::kShardVersionField, &response); return response.obj(); } @@ -959,10 +959,10 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkMigration( if (currentChunk.getShard() == toShard) { // The commit was already done successfully BSONObjBuilder response; - currentCollectionVersion.appendWithField(&response, kCollectionVersionField); + currentCollectionVersion.serializeToBSON(kCollectionVersionField, &response); const auto currentShardVersion = getShardVersion(opCtx, coll, fromShard, currentCollectionVersion); - currentShardVersion.appendWithField(&response, ChunkVersion::kShardVersionField); + currentShardVersion.serializeToBSON(ChunkVersion::kShardVersionField, &response); // Makes sure that the last thing we read in getCurrentChunk, getShardVersion, and // getCollectionVersion gets majority written before to return from this command, otherwise // next RoutingInfo cache refresh from the shard may not see those newest information. @@ -1078,14 +1078,14 @@ StatusWith<BSONObj> ShardingCatalogManager::commitChunkMigration( BSONObjBuilder response; if (!newControlChunk) { // We migrated the last chunk from the donor shard. - newMigratedChunk.getVersion().appendWithField(&response, kCollectionVersionField); + newMigratedChunk.getVersion().serializeToBSON(kCollectionVersionField, &response); const ChunkVersion donorShardVersion( 0, 0, currentCollectionVersion.epoch(), currentCollectionVersion.getTimestamp()); - donorShardVersion.appendWithField(&response, ChunkVersion::kShardVersionField); + donorShardVersion.serializeToBSON(ChunkVersion::kShardVersionField, &response); } else { - newControlChunk.get().getVersion().appendWithField(&response, kCollectionVersionField); - newControlChunk.get().getVersion().appendWithField(&response, - ChunkVersion::kShardVersionField); + newControlChunk.get().getVersion().serializeToBSON(kCollectionVersionField, &response); + newControlChunk.get().getVersion().serializeToBSON(ChunkVersion::kShardVersionField, + &response); } return response.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 d6df635f790..07581ebfbed 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 @@ -101,7 +101,7 @@ TEST_F(CommitChunkMigrate, ChunksUpdatedCorrectly) { validAfter)); // Verify the versions returned match expected values. - auto mver = assertGet(ChunkVersion::parseWithField(versions, "shardVersion")); + auto mver = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); ASSERT_EQ(ChunkVersion(migratedChunk.getVersion().majorVersion() + 1, 1, migratedChunk.getVersion().epoch(), @@ -109,7 +109,7 @@ TEST_F(CommitChunkMigrate, ChunksUpdatedCorrectly) { mver); // Verify that a collection version is returned - auto cver = assertGet(ChunkVersion::parseWithField(versions, "collectionVersion")); + auto cver = ChunkVersion::fromBSONArrayThrowing(versions["collectionVersion"]); ASSERT_TRUE(mver.isOlderOrEqualThan(cver)); // Verify the chunks ended up in the right shards. @@ -182,12 +182,11 @@ TEST_F(CommitChunkMigrate, ChunksUpdatedCorrectlyWithoutControlChunk) { // Verify the version returned matches expected value. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseWithField(versions, "shardVersion"); - ASSERT_OK(mver.getStatus()); - ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver.getValue()); + auto mver = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); + ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver); // Verify that a collection version is returned - auto cver = assertGet(ChunkVersion::parseWithField(versions, "collectionVersion")); + auto cver = ChunkVersion::fromBSONArrayThrowing(versions["collectionVersion"]); ASSERT_EQ(ChunkVersion(origMajorVersion + 1, 0, collEpoch, collTimestamp), cver); // Verify the chunk ended up in the right shard. @@ -248,9 +247,8 @@ TEST_F(CommitChunkMigrate, CheckCorrectOpsCommandNoCtlTrimHistory) { // Verify the version returned matches expected value. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseWithField(versions, "shardVersion"); - ASSERT_OK(mver.getStatus()); - ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver.getValue()); + auto mver = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); + ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver); // Verify the chunk ended up in the right shard. auto chunkDoc0 = @@ -537,9 +535,8 @@ TEST_F(CommitChunkMigrate, CommitWithLastChunkOnShardShouldNotAffectOtherChunks) // Verify the versions returned match expected values. BSONObj versions = resultBSON.getValue(); - auto mver = ChunkVersion::parseWithField(versions, "shardVersion"); - ASSERT_OK(mver.getStatus()); - ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver.getValue()); + auto mver = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); + ASSERT_EQ(ChunkVersion(0, 0, origVersion.epoch(), origVersion.getTimestamp()), mver); // Verify the chunks ended up in the right shards. auto chunkDoc0 = 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 2d681f93e09..907455cf765 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 @@ -96,8 +96,8 @@ TEST_F(MergeChunkTest, MergeExistingChunksCorrectlyShouldSucceed) { ->commitChunksMerge( operationContext(), _nss1, collUuid, rangeToBeMerged, _shardId, validAfter)); - auto collVersion = assertGet(ChunkVersion::parseWithField(versions, "collectionVersion")); - auto shardVersion = assertGet(ChunkVersion::parseWithField(versions, "shardVersion")); + auto collVersion = ChunkVersion::fromBSONArrayThrowing(versions["collectionVersion"]); + auto shardVersion = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); ASSERT_TRUE(origVersion.isOlderThan(shardVersion)); ASSERT_EQ(collVersion, shardVersion); diff --git a/src/mongo/db/s/config/sharding_catalog_manager_split_chunk_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_split_chunk_test.cpp index d93ece922eb..16e59054f76 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_split_chunk_test.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_split_chunk_test.cpp @@ -90,8 +90,8 @@ TEST_F(SplitChunkTest, SplitExistingChunkCorrectlyShouldSucceed) { splitPoints, "shard0000", false /* fromChunkSplitter*/)); - auto collVersion = assertGet(ChunkVersion::parseWithField(versions, "collectionVersion")); - auto shardVersion = assertGet(ChunkVersion::parseWithField(versions, "shardVersion")); + auto collVersion = ChunkVersion::fromBSONArrayThrowing(versions["collectionVersion"]); + auto shardVersion = ChunkVersion::fromBSONArrayThrowing(versions["shardVersion"]); ASSERT_TRUE(origVersion.isOlderThan(shardVersion)); ASSERT_EQ(collVersion, shardVersion); diff --git a/src/mongo/db/s/merge_chunks_command.cpp b/src/mongo/db/s/merge_chunks_command.cpp index 27c9d20b63c..ba3079903f1 100644 --- a/src/mongo/db/s/merge_chunks_command.cpp +++ b/src/mongo/db/s/merge_chunks_command.cpp @@ -145,8 +145,8 @@ void mergeChunks(OperationContext* opCtx, boost::optional<ChunkVersion> shardVersionReceived = [&]() -> boost::optional<ChunkVersion> { // old versions might not have the shardVersion field if (cmdResponse.response[ChunkVersion::kShardVersionField]) { - return uassertStatusOK(ChunkVersion::parseWithField(cmdResponse.response, - ChunkVersion::kShardVersionField)); + return ChunkVersion::fromBSONArrayThrowing( + cmdResponse.response[ChunkVersion::kShardVersionField]); } return boost::none; }(); diff --git a/src/mongo/db/s/op_observer_sharding_test.cpp b/src/mongo/db/s/op_observer_sharding_test.cpp index 60722206eb2..3a64f193175 100644 --- a/src/mongo/db/s/op_observer_sharding_test.cpp +++ b/src/mongo/db/s/op_observer_sharding_test.cpp @@ -49,7 +49,7 @@ void setCollectionFilteringMetadata(OperationContext* opCtx, CollectionMetadata ->setFilteringMetadata(opCtx, std::move(metadata)); BSONObjBuilder builder; - version.appendWithField(&builder, ChunkVersion::kShardVersionField); + version.serializeToBSON(ChunkVersion::kShardVersionField, &builder); auto& oss = OperationShardingState::get(opCtx); oss.initializeClientRoutingVersionsFromCommand(kTestNss, builder.obj()); } diff --git a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.h b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.h index bff5c4c0aae..d212413bcee 100644 --- a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.h +++ b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.h @@ -130,7 +130,7 @@ protected: if (!OperationShardingState::isOperationVersioned(opCtx)) { const auto version = cm.getVersion(kThisShard.getShardId()); BSONObjBuilder builder; - version.appendWithField(&builder, ChunkVersion::kShardVersionField); + version.serializeToBSON(ChunkVersion::kShardVersionField, &builder); auto& oss = OperationShardingState::get(opCtx); oss.initializeClientRoutingVersionsFromCommand(nss, builder.obj()); diff --git a/src/mongo/db/s/sharding_write_router_bm.cpp b/src/mongo/db/s/sharding_write_router_bm.cpp index e0eaf066050..8f1b5443a80 100644 --- a/src/mongo/db/s/sharding_write_router_bm.cpp +++ b/src/mongo/db/s/sharding_write_router_bm.cpp @@ -157,7 +157,7 @@ std::unique_ptr<CatalogCacheMock> createCatalogCacheMock(OperationContext* opCtx BSONObjBuilder builder; chunkManager.getVersion(originatorShard) - .appendWithField(&builder, ChunkVersion::kShardVersionField); + .serializeToBSON(ChunkVersion::kShardVersionField, &builder); // necessary to set the _shardVersions and _databaseVersions to true. Which is needed to get // `getCollectionDescription` to work OperationShardingState::get(opCtx).initializeClientRoutingVersionsFromCommand(kNss, diff --git a/src/mongo/db/s/split_chunk.cpp b/src/mongo/db/s/split_chunk.cpp index 9ea408b54d3..ba1a1fd217f 100644 --- a/src/mongo/db/s/split_chunk.cpp +++ b/src/mongo/db/s/split_chunk.cpp @@ -178,8 +178,8 @@ StatusWith<boost::optional<ChunkRange>> splitChunk(OperationContext* opCtx, boost::optional<ChunkVersion> shardVersionReceived = [&]() -> boost::optional<ChunkVersion> { // old versions might not have the shardVersion field if (cmdResponse.response[ChunkVersion::kShardVersionField]) { - return uassertStatusOK(ChunkVersion::parseWithField(cmdResponse.response, - ChunkVersion::kShardVersionField)); + return ChunkVersion::fromBSONArrayThrowing( + cmdResponse.response[ChunkVersion::kShardVersionField]); } return boost::none; }(); diff --git a/src/mongo/s/chunk_version.cpp b/src/mongo/s/chunk_version.cpp index 0de0352f3b9..bf6f9010b1e 100644 --- a/src/mongo/s/chunk_version.cpp +++ b/src/mongo/s/chunk_version.cpp @@ -37,20 +37,6 @@ namespace mongo { constexpr StringData ChunkVersion::kShardVersionField; -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."}; - - if (versionElem.type() != Array) - return {ErrorCodes::TypeMismatch, - str::stream() << "Invalid type " << versionElem.type() - << " for shardVersion element. Expected an array"}; - - return fromBSON(versionElem.Obj()); -} - StatusWith<ChunkVersion> ChunkVersion::fromBSON(const BSONObj& obj) { BSONObjIterator it(obj); if (!it.more()) @@ -182,8 +168,8 @@ StatusWith<ChunkVersion> ChunkVersion::parseLegacyWithField(const BSONObj& obj, return version; } -void ChunkVersion::appendWithField(BSONObjBuilder* out, StringData field) const { - BSONArrayBuilder arr(out->subarrayStart(field)); +void ChunkVersion::serializeToBSON(StringData field, BSONObjBuilder* builder) const { + BSONArrayBuilder arr(builder->subarrayStart(field)); arr.appendTimestamp(_combined); arr.append(_epoch); arr.append(_timestamp); diff --git a/src/mongo/s/chunk_version.h b/src/mongo/s/chunk_version.h index 7f62f1a9ca6..08bc8efffdc 100644 --- a/src/mongo/s/chunk_version.h +++ b/src/mongo/s/chunk_version.h @@ -62,13 +62,6 @@ public: ChunkVersion() : ChunkVersion(0, 0, OID(), Timestamp()) {} /** - * 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> parseWithField(const BSONObj& obj, StringData field); - - /** * Parses 'obj', which is expected to have three elements: the major/minor versions, the object * id, and the timestamp. The field names don't matter, so 'obj' can be a BSONArray. */ @@ -217,27 +210,22 @@ public: } /** - * Serializes the version held by this object to 'out' in the form: - * { ..., <field>: [ <combined major/minor>, <OID epoch> ], ... }. - */ - void appendWithField(BSONObjBuilder* out, StringData field) const; - - /** - * NOTE: This format is being phased out. Use appendWithField instead. + * NOTE: This format is being phased out. Use serializeToBSON instead. * * Serializes the version held by this object to 'out' in the legacy form: - * { ..., <field>: [ <combined major/minor> ], <field>Epoch: [ <OID epoch> ], ... } + * { ..., <field>: [ <combined major/minor> ], + * <field>Epoch: [ <OID epoch> ], + * <field>Timestamp: [ <Timestamp> ] ... } */ void appendLegacyWithField(BSONObjBuilder* out, StringData field) const; BSONObj toBSON() const; /** - * Same as ChunkVersion::appendWithField adapted for IDL + * Serializes the version held by this object to 'out' in the form: + * { ..., <field>: [ <combined major/minor>, <OID epoch>, <Timestamp> ], ... }. */ - void serializeToBSON(StringData fieldName, BSONObjBuilder* builder) const { - appendWithField(builder, fieldName); - } + void serializeToBSON(StringData field, BSONObjBuilder* builder) const; std::string toString() const; diff --git a/src/mongo/s/chunk_version_test.cpp b/src/mongo/s/chunk_version_test.cpp index e3095e0186f..d07eba0e735 100644 --- a/src/mongo/s/chunk_version_test.cpp +++ b/src/mongo/s/chunk_version_test.cpp @@ -41,13 +41,11 @@ using unittest::assertGet; TEST(ChunkVersionParsing, ToFromBSONRoundtrip) { ChunkVersion version(1, 2, OID::gen(), Timestamp(42)); - const auto roundTripVersion = assertGet(ChunkVersion::parseWithField( - [&] { - BSONObjBuilder builder; - version.appendWithField(&builder, "testVersionField"); - return builder.obj(); - }(), - "testVersionField")); + const auto roundTripVersion = ChunkVersion::fromBSONArrayThrowing([&] { + BSONObjBuilder builder; + version.serializeToBSON("testVersionField", &builder); + return builder.obj(); + }()["testVersionField"]); ASSERT_EQ(version, roundTripVersion); } @@ -66,20 +64,19 @@ TEST(ChunkVersionParsing, ToFromBSONLegacyRoundtrip) { } TEST(ChunkVersionParsing, FromBSONMissingTimestamp) { - ASSERT_THROWS_CODE( - uassertStatusOK(ChunkVersion::parseWithField( - BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3) << OID::gen())), - "testVersionField")), - AssertionException, - ErrorCodes::TypeMismatch); + ASSERT_THROWS_CODE(ChunkVersion::fromBSONArrayThrowing( + BSON("testVersionField" << BSON_ARRAY( + Timestamp(Seconds(2), 3) << OID::gen()))["testVersionField"]), + DBException, + ErrorCodes::TypeMismatch); } TEST(ChunkVersionParsing, FromBSON) { const OID oid = OID::gen(); const Timestamp timestamp(42); - ChunkVersion chunkVersionComplete = assertGet(ChunkVersion::parseWithField( - BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3) << oid << timestamp)), - "testVersionField")); + ChunkVersion chunkVersionComplete = ChunkVersion::fromBSONArrayThrowing( + BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3) + << oid << timestamp))["testVersionField"]); ASSERT(chunkVersionComplete.epoch().isSet()); ASSERT_EQ(oid, chunkVersionComplete.epoch()); @@ -90,17 +87,16 @@ TEST(ChunkVersionParsing, FromBSON) { TEST(ChunkVersionParsing, FromBSONMissingEpoch) { ASSERT_THROWS_CODE( - uassertStatusOK(ChunkVersion::parseWithField( - BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3))), "testVersionField")), - AssertionException, + ChunkVersion::fromBSONArrayThrowing( + BSON("testVersionField" << BSON_ARRAY(Timestamp(Seconds(2), 3)))["testVersionField"]), + DBException, ErrorCodes::TypeMismatch); } TEST(ChunkVersionParsing, FromBSONMissingMajorAndMinor) { - const OID oid = OID::gen(); - ASSERT_THROWS_CODE(uassertStatusOK(ChunkVersion::parseWithField(BSON("testVersionField" << oid), - "testVersionField")), - AssertionException, + ASSERT_THROWS_CODE(ChunkVersion::fromBSONArrayThrowing( + BSON("testVersionField" << OID::gen())["testVersionField"]), + DBException, ErrorCodes::TypeMismatch); } diff --git a/src/mongo/s/cluster_commands_helpers.cpp b/src/mongo/s/cluster_commands_helpers.cpp index be7ee8377d8..35b351556da 100644 --- a/src/mongo/s/cluster_commands_helpers.cpp +++ b/src/mongo/s/cluster_commands_helpers.cpp @@ -245,7 +245,7 @@ BSONObj appendDbVersionIfPresent(BSONObj cmdObj, DatabaseVersion dbVersion) { BSONObj appendShardVersion(BSONObj cmdObj, ChunkVersion version) { BSONObjBuilder cmdWithVersionBob(std::move(cmdObj)); - version.appendWithField(&cmdWithVersionBob, ChunkVersion::kShardVersionField); + version.serializeToBSON(ChunkVersion::kShardVersionField, &cmdWithVersionBob); return cmdWithVersionBob.obj(); } diff --git a/src/mongo/s/commands/cluster_merge_chunks_cmd.cpp b/src/mongo/s/commands/cluster_merge_chunks_cmd.cpp index 4a4c5dd85cd..55a8dc1cb04 100644 --- a/src/mongo/s/commands/cluster_merge_chunks_cmd.cpp +++ b/src/mongo/s/commands/cluster_merge_chunks_cmd.cpp @@ -158,7 +158,7 @@ public: remoteCmdObjB.append(ClusterMergeChunksCommand::shardNameField(), firstChunk.getShardId().toString()); remoteCmdObjB.append("epoch", shardVersion.epoch()); - shardVersion.appendWithField(&remoteCmdObjB, ChunkVersion::kShardVersionField); + shardVersion.serializeToBSON(ChunkVersion::kShardVersionField, &remoteCmdObjB); BSONObj remoteResult; diff --git a/src/mongo/s/commands/cluster_split_cmd.cpp b/src/mongo/s/commands/cluster_split_cmd.cpp index bf4e4e2eeee..69d25edd3ad 100644 --- a/src/mongo/s/commands/cluster_split_cmd.cpp +++ b/src/mongo/s/commands/cluster_split_cmd.cpp @@ -64,7 +64,7 @@ BSONObj selectMedianKey(OperationContext* opCtx, cmd.append("keyPattern", shardKeyPattern.toBSON()); chunkRange.append(&cmd); cmd.appendBool("force", true); - chunkVersion.appendWithField(&cmd, ChunkVersion::kShardVersionField); + chunkVersion.serializeToBSON(ChunkVersion::kShardVersionField, &cmd); auto shard = uassertStatusOK(Grid::get(opCtx)->shardRegistry()->getShard(opCtx, shardId)); diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp index 726dddde7bd..af29d47f6d7 100644 --- a/src/mongo/s/query/cluster_find.cpp +++ b/src/mongo/s/query/cluster_find.cpp @@ -183,10 +183,10 @@ std::vector<std::pair<ShardId, BSONObj>> constructRequestsForShards( findCommandToForward->serialize(BSONObj(), &cmdBuilder); if (cm.isSharded()) { - cm.getVersion(shardId).appendWithField(&cmdBuilder, ChunkVersion::kShardVersionField); + cm.getVersion(shardId).serializeToBSON(ChunkVersion::kShardVersionField, &cmdBuilder); } else if (!query.nss().isOnInternalDb()) { - ChunkVersion::UNSHARDED().appendWithField(&cmdBuilder, - ChunkVersion::kShardVersionField); + ChunkVersion::UNSHARDED().serializeToBSON(ChunkVersion::kShardVersionField, + &cmdBuilder); cmdBuilder.append("databaseVersion", cm.dbVersion().toBSON()); } 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 5a95441405d..8705e60b1de 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 @@ -124,14 +124,12 @@ StatusWith<CommitChunkMigrationRequest> CommitChunkMigrationRequest::createFromC request._toShard = std::move(toShard.getValue()); } - { - auto statusWithChunkVersion = - ChunkVersion::parseWithField(obj, kFromShardCollectionVersion); - if (!statusWithChunkVersion.isOK()) { - return statusWithChunkVersion.getStatus(); - } - - request._collectionEpoch = statusWithChunkVersion.getValue().epoch(); + try { + auto fromShardVersion = + ChunkVersion::fromBSONArrayThrowing(obj[kFromShardCollectionVersion]); + request._collectionEpoch = fromShardVersion.epoch(); + } catch (const DBException& ex) { + return ex.toStatus(); } { @@ -172,7 +170,7 @@ void CommitChunkMigrationRequest::appendAsCommand(BSONObjBuilder* builder, migrateChunk.append(ChunkType::lastmod() + "Timestamp", migratedChunk.getVersion().getTimestamp()); } - fromShardCollectionVersion.appendWithField(builder, kFromShardCollectionVersion); + fromShardCollectionVersion.serializeToBSON(kFromShardCollectionVersion, builder); builder->append(kValidAfter, validAfter); } diff --git a/src/mongo/s/request_types/merge_chunk_request.idl b/src/mongo/s/request_types/merge_chunk_request.idl index 86676b20446..51b122717df 100644 --- a/src/mongo/s/request_types/merge_chunk_request.idl +++ b/src/mongo/s/request_types/merge_chunk_request.idl @@ -45,9 +45,9 @@ types: my_uuid: bson_serialization_type: object description: "A UUID for some reason not generic uuid type." - cpp_type: "mongo::UUID" - deserializer: "mongo::UUID::parse" - serializer: "mongo::UUID::toBSON" + cpp_type: UUID + deserializer: UUID::parse + serializer: UUID::toBSON # serialize [<major/minor>, <epoch>, <timestamp>] # equivalent to using ChunkVersion::appendToCommand / ChunkVersion::parseFromCommand diff --git a/src/mongo/s/shard_util.cpp b/src/mongo/s/shard_util.cpp index 4842f0c1a86..21c212e0100 100644 --- a/src/mongo/s/shard_util.cpp +++ b/src/mongo/s/shard_util.cpp @@ -232,7 +232,7 @@ StatusWith<boost::optional<ChunkRange>> splitChunkAtMultiplePoints( cmd.append("from", shardId.toString()); cmd.append("keyPattern", shardKeyPattern.toBSON()); cmd.append("epoch", epoch); - shardVersion.appendWithField(&cmd, ChunkVersion::kShardVersionField); + shardVersion.serializeToBSON(ChunkVersion::kShardVersionField, &cmd); chunkRange.append(&cmd); cmd.append("splitKeys", splitPoints); diff --git a/src/mongo/s/write_ops/batched_command_request.cpp b/src/mongo/s/write_ops/batched_command_request.cpp index 34c56e605c9..182e55c710e 100644 --- a/src/mongo/s/write_ops/batched_command_request.cpp +++ b/src/mongo/s/write_ops/batched_command_request.cpp @@ -188,7 +188,7 @@ void BatchedCommandRequest::setWriteCommandRequestBase( void BatchedCommandRequest::serialize(BSONObjBuilder* builder) const { _visit([&](auto&& op) { op.serialize({}, builder); }); if (_shardVersion) { - _shardVersion->appendWithField(builder, ChunkVersion::kShardVersionField); + _shardVersion->serializeToBSON(ChunkVersion::kShardVersionField, builder); } if (_dbVersion) { |