diff options
author | Jason Zhang <jason.zhang@mongodb.com> | 2021-02-24 21:14:50 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-03-02 00:29:51 +0000 |
commit | 8c248bf16a781bd096f1829ab61bed9f5655ad41 (patch) | |
tree | 96d4f9bcfa19f1516280ed24cf96068dc93d8d17 | |
parent | a22cbaba0634e6d20244b75c94e8e693e961399d (diff) | |
download | mongo-8c248bf16a781bd096f1829ab61bed9f5655ad41.tar.gz |
SERVER-51774 Ensure the donor's
TenantMigrationCommitted/TenantMigrationAborted write errors don't
exceed the max BSON size
13 files changed, 103 insertions, 170 deletions
diff --git a/buildscripts/resmokeconfig/suites/tenant_migration_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/tenant_migration_jscore_passthrough.yml index 279e27d4fdd..0292ba71536 100644 --- a/buildscripts/resmokeconfig/suites/tenant_migration_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/tenant_migration_jscore_passthrough.yml @@ -87,11 +87,6 @@ selector: - jstests/core/notablescan_capped.js # captrunc command is not blocked during tenant migration. - jstests/core/capped6.js - # TODO (SERVER-51774): Ensure the donor's TenantMigrationCommitted/TenantMigrationAborted write - # errors don't exceed the max BSON size. - - jstests/core/batch_write_command_delete.js - - jstests/core/batch_write_command_insert.js - - jstests/core/batch_write_command_update.js # Multi-updates that conflict with tenant migration are not retried by inject_tenant_prefix.js. - jstests/core/batch_write_collation_estsize.js - jstests/core/bulk_api_ordered.js diff --git a/jstests/replsets/tenant_migration_concurrent_bulk_writes.js b/jstests/replsets/tenant_migration_concurrent_bulk_writes.js index e5674341749..5a1046d8d7f 100644 --- a/jstests/replsets/tenant_migration_concurrent_bulk_writes.js +++ b/jstests/replsets/tenant_migration_concurrent_bulk_writes.js @@ -171,10 +171,6 @@ function retryFailedWrites(primaryDB, collName, writeErrors, ops) { } else { assert(!err.errmsg); } - - assert.eq(err.errInfo.recipientConnectionString, - tenantMigrationTest.getRecipientConnString()); - assert.eq(err.errInfo.tenantId, tenantId); }); tenantMigrationTest.stop(); @@ -255,10 +251,6 @@ function retryFailedWrites(primaryDB, collName, writeErrors, ops) { } else { assert.eq(err.errmsg, ""); } - - assert.eq(err.errInfo.recipientConnectionString, - tenantMigrationTest.getRecipientConnString()); - assert.eq(err.errInfo.tenantId, tenantId); }); tenantMigrationTest.stop(); @@ -406,9 +398,6 @@ function retryFailedWrites(primaryDB, collName, writeErrors, ops) { // blocking writes. assert.eq(writeErrors[0].index, kNumWriteBatchesWithoutMigrationConflict * kMaxBatchSize); assert.eq(writeErrors[0].code, ErrorCodes.TenantMigrationCommitted); - assert.eq(writeErrors[0].errInfo.recipientConnectionString, - tenantMigrationTest.getRecipientConnString()); - assert.eq(writeErrors[0].errInfo.tenantId, tenantId); tenantMigrationTest.stop(); donorRst.stopSet(); @@ -484,9 +473,6 @@ function retryFailedWrites(primaryDB, collName, writeErrors, ops) { // blocking writes. assert.eq(writeErrors[0].index, kNumWriteBatchesWithoutMigrationConflict * kMaxBatchSize); assert.eq(writeErrors[0].code, ErrorCodes.TenantMigrationCommitted); - assert.eq(writeErrors[0].errInfo.recipientConnectionString, - tenantMigrationTest.getRecipientConnString()); - assert.eq(writeErrors[0].errInfo.tenantId, tenantId); tenantMigrationTest.stop(); donorRst.stopSet(); diff --git a/jstests/replsets/tenant_migration_test_max_bson_limit.js b/jstests/replsets/tenant_migration_test_max_bson_limit.js new file mode 100644 index 00000000000..bcebdd4ace1 --- /dev/null +++ b/jstests/replsets/tenant_migration_test_max_bson_limit.js @@ -0,0 +1,95 @@ +/** + * Tests that large write error results from bulk write operations are within the BSON size limit. + * + * Tenant migrations are not expected to be run on servers with ephemeralForTest. + * + * @tags: [requires_fcv_47, requires_majority_read_concern, incompatible_with_eft, + * incompatible_with_windows_tls] + */ +(function() { +'use strict'; + +load("jstests/libs/fail_point_util.js"); +load("jstests/libs/parallelTester.js"); +load("jstests/libs/uuid_util.js"); +load("jstests/replsets/libs/tenant_migration_test.js"); +load("jstests/replsets/libs/tenant_migration_util.js"); + +const kCollName = "testColl"; +const kTenantDefinedDbName = "0"; + +function bulkWriteDocsUnordered(primaryHost, dbName, collName, numDocs) { + const primary = new Mongo(primaryHost); + let primaryDB = primary.getDB(dbName); + + let batch = []; + for (let i = 0; i < numDocs; ++i) { + batch.push({x: i}); + } + + let request = {insert: collName, documents: batch, writeConcern: {w: 1}, ordered: false}; + res = assert.commandFailedWithCode(primaryDB[collName].runCommand(request), + ErrorCodes.TenantMigrationCommitted); + + return res; +} + +jsTestLog("Testing that large write errors fit within the BSON size limit."); + +const tenantMigrationTest = new TenantMigrationTest({name: jsTestName()}); +if (!tenantMigrationTest.isFeatureFlagEnabled()) { + jsTestLog("Skipping test because the tenant migrations feature flag is disabled"); + return; +} + +const tenantId = "bulkUnorderedInserts-committed"; +const migrationOpts = { + migrationIdString: extractUUIDFromObject(UUID()), + tenantId, +}; + +const dbName = tenantMigrationTest.tenantDB(tenantId, kTenantDefinedDbName); +const primary = tenantMigrationTest.getDonorPrimary(); +const primaryDB = primary.getDB(dbName); +const numWriteOps = + assert.commandWorked(primaryDB.hello()).maxWriteBatchSize; // num of writes to run in bulk. + +assert.commandWorked(primaryDB.runCommand({create: kCollName})); + +// Do a large unordered bulk insert that fails all inserts in order to generate a large write +// result. +const writeFp = configureFailPoint(primaryDB, "hangDuringBatchInsert"); +const bulkWriteThread = + new Thread(bulkWriteDocsUnordered, primary.host, dbName, kCollName, numWriteOps); + +bulkWriteThread.start(); +writeFp.wait(); + +const migrationRes = assert.commandWorked(tenantMigrationTest.runMigration(migrationOpts)); +assert.eq(migrationRes.state, TenantMigrationTest.State.kCommitted); + +writeFp.off(); +bulkWriteThread.join(); + +const bulkWriteRes = bulkWriteThread.returnData(); +const writeErrors = bulkWriteRes.writeErrors; + +assert.gt(writeErrors.length, 0); + +writeErrors.forEach((err, arrIndex) => { + assert.eq(err.code, ErrorCodes.TenantMigrationCommitted); + if (arrIndex == 0) { + assert(err.errmsg); + } else { + assert(!err.errmsg); + } +}); + +// This assert is more or less a sanity check since jsThreads need to convert data it returns +// into a BSON object. So if we have reached this assert, we already know that the write result +// is within the BSON limits. +assert.lte(Object.bsonsize(bulkWriteRes), + assert.commandWorked(primaryDB.hello()).maxBsonObjectSize); + +tenantMigrationTest.stop(); +})(); diff --git a/src/mongo/base/error_codes.yml b/src/mongo/base/error_codes.yml index 4b662719e74..4f2ff964a5a 100644 --- a/src/mongo/base/error_codes.yml +++ b/src/mongo/base/error_codes.yml @@ -387,7 +387,7 @@ error_codes: - {code: 319,name: MovePrimaryInProgress} - {code: 320, name: TenantMigrationConflict,extra: TenantMigrationConflictInfo,categories: [TenantMigrationError]} - - {code: 321, name: TenantMigrationCommitted,extra: TenantMigrationCommittedInfo,categories: [TenantMigrationError]} + - {code: 321, name: TenantMigrationCommitted, categories: [TenantMigrationError]} - {code: 322, name: APIVersionError, categories: [VersionedAPIError]} - {code: 323, name: APIStrictError, categories: [VersionedAPIError]} diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp index e2ebacbab75..1bea1a2bb7e 100644 --- a/src/mongo/db/catalog/multi_index_block.cpp +++ b/src/mongo/db/catalog/multi_index_block.cpp @@ -51,7 +51,6 @@ #include "mongo/db/query/collection_query_info.h" #include "mongo/db/repl/repl_set_config.h" #include "mongo/db/repl/replication_coordinator.h" -#include "mongo/db/repl/tenant_migration_committed_info.h" #include "mongo/db/repl/tenant_migration_conflict_info.h" #include "mongo/db/storage/storage_options.h" #include "mongo/db/storage/write_unit_of_work.h" diff --git a/src/mongo/db/commands/tenant_migration_donor_cmds.cpp b/src/mongo/db/commands/tenant_migration_donor_cmds.cpp index 19eb886ce20..6814c2a2db5 100644 --- a/src/mongo/db/commands/tenant_migration_donor_cmds.cpp +++ b/src/mongo/db/commands/tenant_migration_donor_cmds.cpp @@ -35,7 +35,6 @@ #include "mongo/db/repl/repl_server_parameters_gen.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/repl/tenant_migration_access_blocker_util.h" -#include "mongo/db/repl/tenant_migration_committed_info.h" #include "mongo/db/repl/tenant_migration_donor_service.h" namespace mongo { @@ -278,8 +277,7 @@ public: auto durableState = donor->getDurableState(opCtx); - uassert(TenantMigrationCommittedInfo(donor->getTenantId().toString(), - donor->getRecipientConnectionString().toString()), + uassert(ErrorCodes::TenantMigrationCommitted, "Tenant migration already committed", durableState.state == TenantMigrationDonorStateEnum::kAborted); } diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index 33f4f719367..8374a5d6c74 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -58,7 +58,6 @@ #include "mongo/db/repl/repl_client_info.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/repl/tenant_migration_access_blocker_registry.h" -#include "mongo/db/repl/tenant_migration_committed_info.h" #include "mongo/db/repl/tenant_migration_conflict_info.h" #include "mongo/db/retryable_writes_stats.h" #include "mongo/db/stats/counters.h" @@ -315,17 +314,8 @@ boost::optional<BSONObj> generateError(OperationContext* opCtx, if (status.reason() != "") { error.append("errmsg", errorMessage(migrationStatus.reason())); } - if (migrationStatus.extraInfo()) { - error.append( - "errInfo", - migrationStatus.template extraInfo<TenantMigrationCommittedInfo>()->toBSON()); - } } else { error.append("code", int(status.code())); - if (status.extraInfo()) { - error.append("errInfo", - status.template extraInfo<TenantMigrationCommittedInfo>()->toBSON()); - } } } else { error.append("code", int(status.code())); diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript index ef4ef9e3413..3158be1f563 100644 --- a/src/mongo/db/repl/SConscript +++ b/src/mongo/db/repl/SConscript @@ -1248,7 +1248,6 @@ env.Library( env.Library( target='tenant_migration_errors', source= [ - 'tenant_migration_committed_info.cpp', 'tenant_migration_conflict_info.cpp', ], LIBDEPS=[ diff --git a/src/mongo/db/repl/tenant_migration_committed_info.cpp b/src/mongo/db/repl/tenant_migration_committed_info.cpp deleted file mode 100644 index bae765c5adb..00000000000 --- a/src/mongo/db/repl/tenant_migration_committed_info.cpp +++ /dev/null @@ -1,63 +0,0 @@ -/** - * Copyright (C) 2020-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#include "mongo/platform/basic.h" - -#include "mongo/db/repl/tenant_migration_committed_info.h" - -#include "mongo/base/init.h" - -namespace mongo { - -namespace { - -MONGO_INIT_REGISTER_ERROR_EXTRA_INFO(TenantMigrationCommittedInfo); - -constexpr StringData kTenantIdFieldName = "tenantId"_sd; -constexpr StringData kRecipientConnetionStringFieldName = "recipientConnectionString"_sd; - -} // namespace - -BSONObj TenantMigrationCommittedInfo::toBSON() const { - BSONObjBuilder bob; - serialize(&bob); - return bob.obj(); -} - -void TenantMigrationCommittedInfo::serialize(BSONObjBuilder* bob) const { - bob->append(kTenantIdFieldName, _tenantId); - bob->append(kRecipientConnetionStringFieldName, _recipientConnString); -} - -std::shared_ptr<const ErrorExtraInfo> TenantMigrationCommittedInfo::parse(const BSONObj& obj) { - return std::make_shared<TenantMigrationCommittedInfo>( - obj[kTenantIdFieldName].String(), obj[kRecipientConnetionStringFieldName].String()); -} - -} // namespace mongo diff --git a/src/mongo/db/repl/tenant_migration_committed_info.h b/src/mongo/db/repl/tenant_migration_committed_info.h deleted file mode 100644 index d458e969bfb..00000000000 --- a/src/mongo/db/repl/tenant_migration_committed_info.h +++ /dev/null @@ -1,63 +0,0 @@ -/** - * Copyright (C) 2020-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#pragma once - -#include "mongo/base/error_extra_info.h" -#include "mongo/bson/bsonobj.h" -#include "mongo/bson/bsonobjbuilder.h" - -namespace mongo { - -class TenantMigrationCommittedInfo final : public ErrorExtraInfo { -public: - static constexpr auto code = ErrorCodes::TenantMigrationCommitted; - - TenantMigrationCommittedInfo(const std::string tenantId, const std::string recipientConnString) - : _tenantId(std::move(tenantId)), _recipientConnString(std::move(recipientConnString)){}; - - const auto& getTenantId() const { - return _tenantId; - } - - const auto& getRecipientConnString() const { - return _recipientConnString; - } - - BSONObj toBSON() const; - void serialize(BSONObjBuilder* bob) const override; - static std::shared_ptr<const ErrorExtraInfo> parse(const BSONObj&); - -private: - std::string _tenantId; - std::string _recipientConnString; -}; -using TenantMigrationCommittedException = ExceptionFor<ErrorCodes::TenantMigrationCommitted>; - -} // namespace mongo diff --git a/src/mongo/db/repl/tenant_migration_conflict_info.h b/src/mongo/db/repl/tenant_migration_conflict_info.h index e2528a4ccdb..aa7ab16fa5e 100644 --- a/src/mongo/db/repl/tenant_migration_conflict_info.h +++ b/src/mongo/db/repl/tenant_migration_conflict_info.h @@ -67,5 +67,6 @@ private: TenantMigrationAccessBlocker::OperationType _operationType; }; using TenantMigrationConflictException = ExceptionFor<ErrorCodes::TenantMigrationConflict>; +using TenantMigrationCommittedException = ExceptionFor<ErrorCodes::TenantMigrationCommitted>; } // namespace mongo diff --git a/src/mongo/db/repl/tenant_migration_donor_access_blocker.cpp b/src/mongo/db/repl/tenant_migration_donor_access_blocker.cpp index 9789ca823ea..242dae39b44 100644 --- a/src/mongo/db/repl/tenant_migration_donor_access_blocker.cpp +++ b/src/mongo/db/repl/tenant_migration_donor_access_blocker.cpp @@ -36,7 +36,6 @@ #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/repl/storage_interface.h" #include "mongo/db/repl/tenant_migration_access_blocker_executor.h" -#include "mongo/db/repl/tenant_migration_committed_info.h" #include "mongo/db/repl/tenant_migration_conflict_info.h" #include "mongo/db/repl/tenant_migration_donor_access_blocker.h" #include "mongo/logv2/log.h" @@ -73,7 +72,7 @@ Status TenantMigrationDonorAccessBlocker::checkIfCanWrite() { return {TenantMigrationConflictInfo(_tenantId, shared_from_this()), "Write must block until this tenant migration commits or aborts"}; case State::kReject: - return {TenantMigrationCommittedInfo(_tenantId, _recipientConnString), + return {ErrorCodes::TenantMigrationCommitted, "Write must be re-routed to the new owner of this tenant"}; default: MONGO_UNREACHABLE; @@ -150,8 +149,7 @@ SharedSemiFuture<void> TenantMigrationDonorAccessBlocker::_getCanDoClusterTimeRe if (_state == State::kReject) { return SharedSemiFuture<void>( Status(ErrorCodes::TenantMigrationCommitted, - "Read must be re-routed to the new owner of this tenant", - TenantMigrationCommittedInfo(_tenantId, _recipientConnString).toBSON())); + "Read must be re-routed to the new owner of this tenant")); } _stats.numBlockedReads.addAndFetch(1); @@ -162,7 +160,7 @@ Status TenantMigrationDonorAccessBlocker::checkIfLinearizableReadWasAllowed( OperationContext* opCtx) { stdx::lock_guard<Latch> lg(_mutex); if (_state == State::kReject) { - return {TenantMigrationCommittedInfo(_tenantId, _recipientConnString), + return {ErrorCodes::TenantMigrationCommitted, "Read must be re-routed to the new owner of this tenant"}; } return Status::OK(); @@ -177,7 +175,7 @@ Status TenantMigrationDonorAccessBlocker::checkIfCanBuildIndex() { return {TenantMigrationConflictInfo(_tenantId, shared_from_this(), kIndexBuild), "Index build must block until tenant migration is committed or aborted."}; case State::kReject: - return {TenantMigrationCommittedInfo(_tenantId, _recipientConnString), + return {ErrorCodes::TenantMigrationCommitted, "Index build must be re-routed to the new owner of this tenant"}; case State::kAborted: return Status::OK(); @@ -311,8 +309,7 @@ void TenantMigrationDonorAccessBlocker::_onMajorityCommitCommitOpTime( _state = State::kReject; Status error{ErrorCodes::TenantMigrationCommitted, - "Write or read must be re-routed to the new owner of this tenant", - TenantMigrationCommittedInfo(_tenantId, _recipientConnString).toBSON()}; + "Write or read must be re-routed to the new owner of this tenant"}; _completionPromise.setError(error); _transitionOutOfBlockingPromise.setFrom(error); diff --git a/src/mongo/db/repl/tenant_migration_recipient_access_blocker.cpp b/src/mongo/db/repl/tenant_migration_recipient_access_blocker.cpp index 1704c91dc77..ce574c90f1d 100644 --- a/src/mongo/db/repl/tenant_migration_recipient_access_blocker.cpp +++ b/src/mongo/db/repl/tenant_migration_recipient_access_blocker.cpp @@ -36,7 +36,6 @@ #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/repl/storage_interface.h" #include "mongo/db/repl/tenant_migration_access_blocker_executor.h" -#include "mongo/db/repl/tenant_migration_committed_info.h" #include "mongo/db/repl/tenant_migration_recipient_access_blocker.h" #include "mongo/logv2/log.h" #include "mongo/util/cancelation.h" |