diff options
| author | Christopher Caplinger <christopher.caplinger@mongodb.com> | 2022-08-15 17:59:10 +0000 |
|---|---|---|
| committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-08-15 19:25:35 +0000 |
| commit | e455a0fa9c571753bf07d4c951bfbf881005c3a7 (patch) | |
| tree | aaf4cbecd9251f68b6ddaac2513b11e7b55da9f6 | |
| parent | 2c701bf03543d9feaad6ba3faf7851bb26d0ee30 (diff) | |
| download | mongo-e455a0fa9c571753bf07d4c951bfbf881005c3a7.tar.gz | |
SERVER-63454: Don't require tenantId for shard merge
46 files changed, 546 insertions, 394 deletions
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml index 56eb924b341..3c1f72b17d4 100644 --- a/etc/backports_required_for_multiversion_tests.yml +++ b/etc/backports_required_for_multiversion_tests.yml @@ -194,6 +194,8 @@ last-continuous: test_file: jstests/replsets/apply_ops_dropDatabase.js - ticket: SERVER-63732 test_file: jstests/sharding/shard_collection_basic.js + - ticket: SERVER-63454 + test_file: jstests/replsets/tenant_migration_shard_merge_invalid_inputs.js - ticket: SERVER-6491 test_file: jstests/sharding/shard_key_index_must_exist.js - ticket: SERVER-64142 @@ -617,6 +619,8 @@ last-lts: test_file: jstests/replsets/tenant_migration_recipient_startup_recovery.js - ticket: SERVER-63122 test_file: jstests/replsets/tenant_migration_network_error_via_rollback.js + - ticket: SERVER-63454 + test_file: jstests/replsets/tenant_migration_shard_merge_invalid_inputs.js - ticket: SERVER-63732 test_file: jstests/sharding/shard_collection_basic.js - ticket: SERVER-6491 diff --git a/jstests/auth/lib/commands_lib.js b/jstests/auth/lib/commands_lib.js index e809bcbc427..99ef42720d5 100644 --- a/jstests/auth/lib/commands_lib.js +++ b/jstests/auth/lib/commands_lib.js @@ -3704,7 +3704,7 @@ var authCommandsLib = { command: { donorStartMigration: 1, protocol: isShardMergeEnabled ? "shard merge" : "multitenant migrations", - tenantId: "testTenantId", + tenantId: isShardMergeEnabled ? "" : "testTenantId", migrationId: UUID(), recipientConnectionString: "recipient-rs/localhost:1234", readPreference: {mode: "primary"}, @@ -3730,7 +3730,8 @@ var authCommandsLib = { recipientSyncData: 1, migrationId: UUID(), donorConnectionString: "donor-rs/localhost:1234", - tenantId: "testTenantId", + protocol: isShardMergeEnabled ? "shard merge" : "multitenant migrations", + tenantId: isShardMergeEnabled ? "" : "testTenantId", readPreference: {mode: "primary"}, startMigrationDonorTimestamp: Timestamp(1, 1), recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, @@ -3754,7 +3755,8 @@ var authCommandsLib = { recipientForgetMigration: 1, migrationId: UUID(), donorConnectionString: "donor-rs/localhost:1234", - tenantId: "testTenantId", + protocol: isShardMergeEnabled ? "shard merge" : "multitenant migrations", + tenantId: isShardMergeEnabled ? "" : "testTenantId", readPreference: {mode: "primary"}, }, skipSharded: true, diff --git a/jstests/replsets/libs/tenant_migration_test.js b/jstests/replsets/libs/tenant_migration_test.js index e1d4b472dff..04619679e33 100644 --- a/jstests/replsets/libs/tenant_migration_test.js +++ b/jstests/replsets/libs/tenant_migration_test.js @@ -204,10 +204,9 @@ function TenantMigrationTest({ this.waitForMigrationToComplete = function( migrationOpts, retryOnRetryableErrors = false, forgetMigration = false) { // Assert that the migration has already been started. - const tenantId = migrationOpts.tenantId; - assert(this.getDonorPrimary() - .getCollection(TenantMigrationTest.kConfigDonorsNS) - .findOne({tenantId})); + assert(this.getDonorPrimary().getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ + _id: UUID(migrationOpts.migrationIdString) + })); const donorStartReply = this.runDonorStartMigration( migrationOpts, {waitForMigrationToComplete: true, retryOnRetryableErrors}); @@ -246,12 +245,12 @@ function TenantMigrationTest({ const cmdObj = { donorStartMigration: 1, - tenantId, migrationId: UUID(migrationIdString), + tenantId, recipientConnectionString, readPreference, donorCertificateForRecipient, - recipientCertificateForDonor + recipientCertificateForDonor, }; const stateRes = TenantMigrationUtil.runTenantMigrationCommand(cmdObj, this.getDonorRst(), { @@ -440,7 +439,7 @@ function TenantMigrationTest({ assert.soon( () => { result = this.isRecipientNodeInExpectedState( - node, migrationId, tenantId, expectedState, expectedAccessState); + {node, migrationId, tenantId, expectedState, expectedAccessState}); return result.value; }, () => { @@ -458,11 +457,16 @@ function TenantMigrationTest({ * Asserts that the migration 'migrationId' and 'tenantId' is in the expected state on all the * given recipient nodes. */ - this.assertRecipientNodesInExpectedState = function( - nodes, migrationId, tenantId, expectedState, expectedAccessState) { + this.assertRecipientNodesInExpectedState = function({ + nodes, + migrationId, + tenantId, + expectedState, + expectedAccessState, + }) { nodes.forEach(node => { let result = this.isRecipientNodeInExpectedState( - node, migrationId, tenantId, expectedState, expectedAccessState); + {node, migrationId, tenantId, expectedState, expectedAccessState}); assert(result.value, () => { return "assertRecipientNodesInExpectedState failed: " + buildErrorMsg(migrationId, @@ -478,8 +482,13 @@ function TenantMigrationTest({ * Returns true if the durable and in-memory state for the migration 'migrationId' and * 'tenantId' is in the expected state, and false otherwise. */ - this.isRecipientNodeInExpectedState = function( - node, migrationId, tenantId, expectedState, expectedAccessState) { + this.isRecipientNodeInExpectedState = function({ + node, + migrationId, + tenantId, + expectedState, + expectedAccessState, + }) { const configRecipientsColl = this.getRecipientPrimary().getCollection("config.tenantMigrationRecipients"); const configDoc = configRecipientsColl.findOne({_id: migrationId}); diff --git a/jstests/replsets/libs/tenant_migration_util.js b/jstests/replsets/libs/tenant_migration_util.js index d5b62fac17f..d0e9354f1c1 100644 --- a/jstests/replsets/libs/tenant_migration_util.js +++ b/jstests/replsets/libs/tenant_migration_util.js @@ -39,9 +39,14 @@ var TenantMigrationUtil = (function() { * flag is enabled. */ function donorStartMigrationWithProtocol(cmd, db) { - // If we don't pass "protocol", the server uses "multitenant migrations" by default. - if (cmd['protocol'] === undefined && isShardMergeEnabled(db)) { - return Object.assign(Object.assign({}, cmd), {protocol: "shard merge"}); + // If we don't pass "protocol" and shard merge is enabled, we set the protocol to + // "shard merge". Otherwise, the provided protocol is used, which defaults to + // "multitenant migrations" if not provided. + if (cmd["protocol"] === undefined && isShardMergeEnabled(db)) { + const cmdCopy = Object.assign({}, cmd); + delete cmdCopy.tenantId; + cmdCopy.protocol = "shard merge"; + return cmdCopy; } return cmd; @@ -139,8 +144,8 @@ var TenantMigrationUtil = (function() { const cmdObj = { donorStartMigration: 1, migrationId: UUID(migrationOpts.migrationIdString), - recipientConnectionString: migrationOpts.recipientConnString, tenantId: migrationOpts.tenantId, + recipientConnectionString: migrationOpts.recipientConnString, readPreference: migrationOpts.readPreference || {mode: "primary"}, donorCertificateForRecipient: migrationOpts.donorCertificateForRecipient || migrationCertificates.donorCertificateForRecipient, diff --git a/jstests/replsets/tenant_migration_advance_stable_ts_after_clone.js b/jstests/replsets/tenant_migration_advance_stable_ts_after_clone.js index 56d75a3ada8..edff60a2879 100644 --- a/jstests/replsets/tenant_migration_advance_stable_ts_after_clone.js +++ b/jstests/replsets/tenant_migration_advance_stable_ts_after_clone.js @@ -26,15 +26,14 @@ const kTenantIdPrefix = "testTenantId"; const kUnrelatedDbNameDonor = "unrelatedDBDonor"; const kUnrelatedDbNameRecipient = "unrelatedDBRecipient"; const collName = "foo"; -const tenantId = kTenantIdPrefix + "-0"; +const tenantId = `${kTenantIdPrefix}-0`; const migrationId = UUID(); const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), - tenantId: tenantId, }; const tmt = new TenantMigrationTest({name: jsTestName()}); -tmt.insertDonorDB(tenantId + "_db", collName); +tmt.insertDonorDB(`${tenantId}_db`, collName); const donorPrimary = tmt.getDonorPrimary(); const recipientPrimary = tmt.getRecipientPrimary(); diff --git a/jstests/replsets/tenant_migration_buildindex_shard_merge.js b/jstests/replsets/tenant_migration_buildindex_shard_merge.js index e18e266ed36..03b51100ca3 100644 --- a/jstests/replsets/tenant_migration_buildindex_shard_merge.js +++ b/jstests/replsets/tenant_migration_buildindex_shard_merge.js @@ -50,11 +50,9 @@ function createIndexShouldFail(primaryHost, dbName, collName, indexSpec) { } const migrationId = UUID(); -// TODO (SERVER-63454): remove tenantId, and remove kTenant2DbName, db2, tenant2IndexThread, etc. const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), recipientConnString: tenantMigrationTest.getRecipientConnString(), - tenantId: kTenant1Id, }; const donorRstArgs = TenantMigrationUtil.createRstArgs(tenantMigrationTest.getDonorRst()); @@ -74,9 +72,6 @@ var initFpCount = .count; const tenant1IndexThread = new Thread(createIndexShouldFail, donorPrimary.host, kTenant1DbName, kNonEmptyCollName, {b: 1}); -// Even though tenantId1 is passed to donorStartMigration, the donor aborts this index too -// because protocol is "shard merge". -// TODO (SERVER-63454): remove comment above. const tenant2IndexThread = new Thread(createIndexShouldFail, donorPrimary.host, kTenant2DbName, kNonEmptyCollName, {y: 1}); tenant1IndexThread.start(); diff --git a/jstests/replsets/tenant_migration_concurrent_migrations.js b/jstests/replsets/tenant_migration_concurrent_migrations.js index 752c18fffca..ef8d7638f68 100644 --- a/jstests/replsets/tenant_migration_concurrent_migrations.js +++ b/jstests/replsets/tenant_migration_concurrent_migrations.js @@ -152,7 +152,7 @@ const kTenantIdPrefix = "testTenantId"; const connPoolStatsBefore = assert.commandWorked(donorPrimary.adminCommand({connPoolStats: 1})); - let blockFp = configureFailPoint( + const blockFp = configureFailPoint( donorPrimary, "pauseTenantMigrationBeforeLeavingBlockingState", {tenantId: tenantId1}); assert.commandWorked(tenantMigrationTest0.startMigration(migrationOpts0)); assert.commandWorked(tenantMigrationTest1.startMigration(migrationOpts1)); diff --git a/jstests/replsets/tenant_migration_concurrent_migrations_stress_test.js b/jstests/replsets/tenant_migration_concurrent_migrations_stress_test.js index 18c41480f70..3f95d39fe99 100644 --- a/jstests/replsets/tenant_migration_concurrent_migrations_stress_test.js +++ b/jstests/replsets/tenant_migration_concurrent_migrations_stress_test.js @@ -1,9 +1,13 @@ /** * Stress test runs many concurrent migrations against the same recipient. + * + * TODO SERVER-61231: shard merge can't handle concurrent migrations. + * * @tags: [ * incompatible_with_amazon_linux, * incompatible_with_macos, * incompatible_with_windows_tls, + * incompatible_with_shard_merge, * requires_majority_read_concern, * requires_persistence, * serverless, @@ -45,14 +49,6 @@ const tenantMigrationTest = new TenantMigrationTest({ const donorPrimary = tenantMigrationTest.getDonorPrimary(); const recipientPrimary = tenantMigrationTest.getRecipientPrimary(); -if (TenantMigrationUtil.isShardMergeEnabled(donorPrimary.getDB("admin"))) { - // This test runs multiple concurrent migrations, which shard merge can't handle. - jsTestLog( - "Skip: featureFlagShardMerge is enabled and this test runs multiple concurrent migrations, which shard merge can't handle."); - tenantMigrationTest.stop(); - return; -} - setLogVerbosity([donorPrimary, recipientPrimary], { "tenantMigration": {"verbosity": 0}, "replication": {"verbosity": 0}, diff --git a/jstests/replsets/tenant_migration_concurrent_reads_on_donor.js b/jstests/replsets/tenant_migration_concurrent_reads_on_donor.js index 8d61e04c981..360c7a4276d 100644 --- a/jstests/replsets/tenant_migration_concurrent_reads_on_donor.js +++ b/jstests/replsets/tenant_migration_concurrent_reads_on_donor.js @@ -125,7 +125,7 @@ function testRejectReadsAfterMigrationCommitted(testCase, dbName, collName) { donorRst.awaitLastOpCommitted(); const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationIdString), }); const nodes = testCase.isSupportedOnSecondaries ? donorRst.nodes : [donorPrimary]; nodes.forEach(node => { @@ -181,7 +181,7 @@ function testDoNotRejectReadsAfterMigrationAborted(testCase, dbName, collName) { donorRst.awaitLastOpCommitted(); const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationIdString), }); const nodes = testCase.isSupportedOnSecondaries ? donorRst.nodes : [donorPrimary]; nodes.forEach(node => { @@ -234,7 +234,7 @@ function testBlockReadsAfterMigrationEnteredBlocking(testCase, dbName, collName) donorRst.awaitLastOpCommitted(); const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationIdString), }); const command = testCase.requiresReadTimestamp ? testCase.command(collName, donorDoc.blockTimestamp) @@ -297,7 +297,7 @@ function testRejectBlockedReadsAfterMigrationCommitted(testCase, dbName, collNam donorRst.awaitLastOpCommitted(); const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationIdString), }); const command = testCase.requiresReadTimestamp ? testCase.command(collName, donorDoc.blockTimestamp) @@ -365,7 +365,7 @@ function testUnblockBlockedReadsAfterMigrationAborted(testCase, dbName, collName donorRst.awaitLastOpCommitted(); const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationIdString), }); const command = testCase.requiresReadTimestamp ? testCase.command(collName, donorDoc.blockTimestamp) diff --git a/jstests/replsets/tenant_migration_concurrent_reads_on_recipient.js b/jstests/replsets/tenant_migration_concurrent_reads_on_recipient.js index 71d12c1f044..0867c371d9f 100644 --- a/jstests/replsets/tenant_migration_concurrent_reads_on_recipient.js +++ b/jstests/replsets/tenant_migration_concurrent_reads_on_recipient.js @@ -131,7 +131,7 @@ function testRejectOnlyReadsWithAtClusterTimeLessThanRejectReadsBeforeTimestamp( const recipientDoc = recipientPrimary.getCollection(TenantMigrationTest.kConfigRecipientsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationOpts.migrationIdString), }); assert.lt(preMigrationTimestamp, recipientDoc.rejectReadsBeforeTimestamp); @@ -276,7 +276,7 @@ function testDoNotRejectReadsAfterMigrationAbortedAfterReachingRejectReadsBefore const recipientDoc = recipientPrimary.getCollection(TenantMigrationTest.kConfigRecipientsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationOpts.migrationIdString), }); const nodes = testCase.isSupportedOnSecondaries ? recipientRst.nodes : [recipientPrimary]; diff --git a/jstests/replsets/tenant_migration_concurrent_writes_on_recipient.js b/jstests/replsets/tenant_migration_concurrent_writes_on_recipient.js index a1196fd3eff..cdd870c6c37 100644 --- a/jstests/replsets/tenant_migration_concurrent_writes_on_recipient.js +++ b/jstests/replsets/tenant_migration_concurrent_writes_on_recipient.js @@ -41,12 +41,12 @@ function cleanup(dbName) { const tenantId = `${kTenantId}Commit`; const donorDB = `${tenantId}_test`; tenantMigrationTest.insertDonorDB(donorDB, "test"); - const ns = tenantId + "_testDb.testColl"; + const ns = `${tenantId}_testDb.testColl`; const tenantCollOnRecipient = recipientPrimary.getCollection(ns); const migrationOpts = { migrationIdString: extractUUIDFromObject(UUID()), - tenantId: tenantId, + tenantId, recipientConnString: tenantMigrationTest.getRecipientConnString(), }; @@ -108,12 +108,12 @@ function cleanup(dbName) { const tenantId = `${kTenantId}AbortBeforeReturnAfterReachingTs`; const donorDB = `${tenantId}_test`; tenantMigrationTest.insertDonorDB(donorDB, "test"); - const ns = tenantId + "_testDb.testColl"; + const ns = `${tenantId}_testDb.testColl`; const tenantCollOnRecipient = recipientPrimary.getCollection(ns); const migrationOpts = { migrationIdString: extractUUIDFromObject(UUID()), - tenantId: tenantId, + tenantId, recipientConnString: tenantMigrationTest.getRecipientConnString(), }; @@ -150,12 +150,12 @@ function cleanup(dbName) { const tenantId = `${kTenantId}AbortAfterReturnAfterReachingTs`; const donorDB = `${tenantId}_test`; tenantMigrationTest.insertDonorDB(donorDB, "test"); - const ns = tenantId + "_testDb.testColl"; + const ns = `${tenantId}_testDb.testColl`; const tenantCollOnRecipient = recipientPrimary.getCollection(ns); const migrationOpts = { migrationIdString: extractUUIDFromObject(UUID()), - tenantId: kTenantId + "AbortAfterReturnAfterReachingTs", + tenantId: `${kTenantId}AbortAfterReturnAfterReachingTs`, recipientConnString: tenantMigrationTest.getRecipientConnString(), }; diff --git a/jstests/replsets/tenant_migration_conflicting_donor_start_migration_cmds.js b/jstests/replsets/tenant_migration_conflicting_donor_start_migration_cmds.js index 053b98c41e4..ed21502a4b4 100644 --- a/jstests/replsets/tenant_migration_conflicting_donor_start_migration_cmds.js +++ b/jstests/replsets/tenant_migration_conflicting_donor_start_migration_cmds.js @@ -16,13 +16,16 @@ load("jstests/libs/uuid_util.js"); load("jstests/replsets/libs/tenant_migration_test.js"); +function getRecipientSyncDataMetrics(recipientPrimary) { + return recipientPrimary.adminCommand({serverStatus: 1}).metrics.commands.recipientSyncData; +} + /** * Asserts that the number of recipientDataSync commands executed on the given recipient primary is * equal to the given number. */ function checkNumRecipientSyncDataCmdExecuted(recipientPrimary, expectedNumExecuted) { - const recipientSyncDataMetrics = - recipientPrimary.adminCommand({serverStatus: 1}).metrics.commands.recipientSyncData; + const recipientSyncDataMetrics = getRecipientSyncDataMetrics(recipientPrimary); assert.eq(0, recipientSyncDataMetrics.failed); assert.eq(expectedNumExecuted, recipientSyncDataMetrics.total); } @@ -54,50 +57,43 @@ function generateUniqueTenantId() { return kTenantIdPrefix + tenantCounter++; } -function setup() { - const {donor: donorNodeOptions} = TenantMigrationUtil.makeX509OptionsForTest(); - donorNodeOptions.setParameter = donorNodeOptions.setParameter || {}; - Object.assign(donorNodeOptions.setParameter, { - tenantMigrationGarbageCollectionDelayMS: 1 * 1000, - ttlMonitorSleepSecs: 1, - }); - const donorRst = new ReplSetTest({ - nodes: 1, - name: 'donorRst', - nodeOptions: donorNodeOptions, - }); - - donorRst.startSet(); - donorRst.initiate(); - - const tenantMigrationTest = - new TenantMigrationTest({name: jsTestName(), donorRst, quickGarbageCollection: true}); - - const donorPrimary = donorRst.getPrimary(); - const recipientPrimary = tenantMigrationTest.getRecipientPrimary(); - - return { - tenantMigrationTest, - donorRst, - donorPrimary, - recipientPrimary, - teardown: function() { - tenantMigrationTest.stop(); - donorRst.stopSet(); - }, - }; +const {donor: donorNodeOptions} = TenantMigrationUtil.makeX509OptionsForTest(); +donorNodeOptions.setParameter = donorNodeOptions.setParameter || {}; +Object.assign(donorNodeOptions.setParameter, { + tenantMigrationGarbageCollectionDelayMS: 1 * 1000, + ttlMonitorSleepSecs: 1, +}); +const donorRst = new ReplSetTest({ + nodes: 1, + name: 'donorRst', + nodeOptions: donorNodeOptions, +}); + +donorRst.startSet(); +donorRst.initiate(); + +const tenantMigrationTest = + new TenantMigrationTest({name: jsTestName(), donorRst, quickGarbageCollection: true}); + +const donorPrimary = donorRst.getPrimary(); +const recipientPrimary = tenantMigrationTest.getRecipientPrimary(); + +function teardown() { + tenantMigrationTest.stop(); + donorRst.stopSet(); } // Test that a retry of a donorStartMigration command joins the existing migration that has // completed but has not been garbage-collected. (() => { const tenantId = `${generateUniqueTenantId()}RetryAfterMigrationCompletes`; + const migrationId = UUID(); const migrationOpts = { - migrationIdString: extractUUIDFromObject(UUID()), + migrationIdString: extractUUIDFromObject(migrationId), tenantId, }; - const {tenantMigrationTest, recipientPrimary, teardown} = setup(); + const recipientSyncDataMetrics = getRecipientSyncDataMetrics(recipientPrimary); TenantMigrationTest.assertCommitted( tenantMigrationTest.runMigration(migrationOpts, {automaticForgetMigration: false})); @@ -106,21 +102,22 @@ function setup() { // If the second donorStartMigration had started a duplicate migration, the recipient would have // received four recipientSyncData commands instead of two. - checkNumRecipientSyncDataCmdExecuted(recipientPrimary, 2); + checkNumRecipientSyncDataCmdExecuted(recipientPrimary, recipientSyncDataMetrics.total + 2); assert.commandWorked(tenantMigrationTest.forgetMigration(migrationOpts.migrationIdString)); - teardown(); + tenantMigrationTest.waitForMigrationGarbageCollection(migrationId, tenantId); })(); // Test that a retry of a donorStartMigration command joins the ongoing migration. (() => { const tenantId = `${generateUniqueTenantId()}RetryBeforeMigrationCompletes`; + const migrationId = UUID(); const migrationOpts = { - migrationIdString: extractUUIDFromObject(UUID()), + migrationIdString: extractUUIDFromObject(migrationId), tenantId, }; - const {tenantMigrationTest, recipientPrimary, teardown} = setup(); + const recipientSyncDataMetrics = getRecipientSyncDataMetrics(recipientPrimary); assert.commandWorked(tenantMigrationTest.startMigration(migrationOpts)); assert.commandWorked(tenantMigrationTest.startMigration(migrationOpts)); @@ -132,10 +129,10 @@ function setup() { // If the second donorStartMigration had started a duplicate migration, the recipient would have // received four recipientSyncData commands instead of two. - checkNumRecipientSyncDataCmdExecuted(recipientPrimary, 2); + checkNumRecipientSyncDataCmdExecuted(recipientPrimary, recipientSyncDataMetrics.total + 2); assert.commandWorked(tenantMigrationTest.forgetMigration(migrationOpts.migrationIdString)); - teardown(); + tenantMigrationTest.waitForMigrationGarbageCollection(migrationId, tenantId); })(); /** @@ -228,6 +225,8 @@ function testConcurrentConflictingMigrations({ tenantMigrationTest0.waitForMigrationToComplete(migrationOpts0)); assert.commandWorked( tenantMigrationTest0.forgetMigration(migrationOpts0.migrationIdString)); + tenantMigrationTest0.waitForMigrationGarbageCollection(migrationOpts0.migrationIdString, + migrationOpts0.tenantId); } else { assert.commandFailedWithCode(res0, ErrorCodes.ConflictingOperationInProgress); assertNoCertificateOrPrivateKey(res0.errmsg); @@ -250,12 +249,13 @@ function testConcurrentConflictingMigrations({ tenantMigrationTest1.waitForMigrationToComplete(migrationOpts1)); assert.commandWorked( tenantMigrationTest1.forgetMigration(migrationOpts1.migrationIdString)); + tenantMigrationTest1.waitForMigrationGarbageCollection(migrationOpts1.migrationIdString, + migrationOpts1.tenantId); } } // Test migrations with different migrationIds but identical settings. (() => { - const {tenantMigrationTest, donorPrimary, teardown} = setup(); const makeTestParams = () => { const migrationOpts0 = { migrationIdString: extractUUIDFromObject(UUID()), @@ -274,14 +274,18 @@ function testConcurrentConflictingMigrations({ testStartingConflictingMigrationAfterInitialMigrationCommitted(makeTestParams()); testConcurrentConflictingMigrations(makeTestParams()); - teardown(); })(); // Test reusing a migrationId for different migration settings. // Test different tenantIds. (() => { - const {tenantMigrationTest, donorPrimary, teardown} = setup(); + if (TenantMigrationUtil.isShardMergeEnabled(donorPrimary.getDB("admin"))) { + jsTestLog( + "Skip: featureFlagShardMerge is enabled and this test tests migrations with different tenant ids."); + return; + } + const makeTestParams = () => { const migrationOpts0 = { migrationIdString: extractUUIDFromObject(UUID()), @@ -300,18 +304,10 @@ function testConcurrentConflictingMigrations({ testStartingConflictingMigrationAfterInitialMigrationCommitted(makeTestParams()); testConcurrentConflictingMigrations(makeTestParams()); - teardown(); })(); // Test different recipient connection strings. (() => { - const { - tenantMigrationTest: tenantMigrationTest0, - donorRst, - donorPrimary, - teardown, - } = setup(); - const tenantMigrationTest1 = new TenantMigrationTest({name: `${jsTestName()}1`, donorRst}); const makeTestParams = () => { @@ -323,7 +319,7 @@ function testConcurrentConflictingMigrations({ // no need to set it here. const migrationOpts1 = Object.extend({}, migrationOpts0, true); return { - tenantMigrationTest0, + tenantMigrationTest0: tenantMigrationTest, migrationOpts0, tenantMigrationTest1, migrationOpts1, @@ -335,13 +331,10 @@ function testConcurrentConflictingMigrations({ testConcurrentConflictingMigrations(makeTestParams()); tenantMigrationTest1.stop(); - teardown(); })(); // Test different cloning read preference. (() => { - const {tenantMigrationTest, donorPrimary, teardown} = setup(); - const makeTestParams = () => { const migrationOpts0 = { migrationIdString: extractUUIDFromObject(UUID()), @@ -360,7 +353,6 @@ function testConcurrentConflictingMigrations({ testStartingConflictingMigrationAfterInitialMigrationCommitted(makeTestParams()); testConcurrentConflictingMigrations(makeTestParams()); - teardown(); })(); const kDonorCertificateAndPrivateKey = @@ -374,8 +366,6 @@ const kExpiredRecipientCertificateAndPrivateKey = TenantMigrationUtil.getCertifi // Test different donor certificates. (() => { - const {tenantMigrationTest, donorPrimary, teardown} = setup(); - const makeTestParams = () => { const migrationOpts0 = { migrationIdString: extractUUIDFromObject(UUID()), @@ -396,13 +386,10 @@ const kExpiredRecipientCertificateAndPrivateKey = TenantMigrationUtil.getCertifi testStartingConflictingMigrationAfterInitialMigrationCommitted(makeTestParams()); testConcurrentConflictingMigrations(makeTestParams()); - teardown(); })(); // Test different recipient certificates. (() => { - const {tenantMigrationTest, donorPrimary, teardown} = setup(); - const makeTestParams = () => { const migrationOpts0 = { migrationIdString: extractUUIDFromObject(UUID()), @@ -423,6 +410,7 @@ const kExpiredRecipientCertificateAndPrivateKey = TenantMigrationUtil.getCertifi testStartingConflictingMigrationAfterInitialMigrationCommitted(makeTestParams()); testConcurrentConflictingMigrations(makeTestParams()); - teardown(); })(); + +teardown(); })(); diff --git a/jstests/replsets/tenant_migration_conflicting_recipient_sync_data_cmds.js b/jstests/replsets/tenant_migration_conflicting_recipient_sync_data_cmds.js index 38399baccf5..a8278e285dc 100644 --- a/jstests/replsets/tenant_migration_conflicting_recipient_sync_data_cmds.js +++ b/jstests/replsets/tenant_migration_conflicting_recipient_sync_data_cmds.js @@ -163,7 +163,7 @@ assert.commandWorked(primary.adminCommand({ // Only one instance should have succeeded in persisting the state doc, other should have failed // with ErrorCodes.ConflictingOperationInProgress. - assert.eq(1, configRecipientsColl.count({tenantId: tenantId})); + assert.eq(1, configRecipientsColl.count({})); // Run another recipientSyncData cmd for the tenant. Since the previous migration hasn't been // garbage collected, the migration is considered as active. So this command should fail with @@ -176,7 +176,7 @@ assert.commandWorked(primary.adminCommand({ assert.commandFailedWithCode(res2, ErrorCodes.ConflictingOperationInProgress); // Collection count should remain the same. - assert.eq(1, configRecipientsColl.count({tenantId: tenantId})); + assert.eq(1, configRecipientsColl.count({})); fpPauseBeforeRunTenantMigrationRecipientInstance.off(); })(); diff --git a/jstests/replsets/tenant_migration_donor_shutdown_while_blocking_reads.js b/jstests/replsets/tenant_migration_donor_shutdown_while_blocking_reads.js index b55ba5cb6a2..f4e2106a07f 100644 --- a/jstests/replsets/tenant_migration_donor_shutdown_while_blocking_reads.js +++ b/jstests/replsets/tenant_migration_donor_shutdown_while_blocking_reads.js @@ -43,7 +43,7 @@ assert.commandWorked(tenantMigrationTest.startMigration(migrationOpts)); fp.wait(); const donorDoc = - donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({tenantId: kTenantId}); + donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({_id: migrationId}); assert.neq(null, donorDoc); let readThread = new Thread((host, dbName, collName, afterClusterTime) => { diff --git a/jstests/replsets/tenant_migration_donor_state_machine.js b/jstests/replsets/tenant_migration_donor_state_machine.js index cd251bb6657..ba1e7d40fc0 100644 --- a/jstests/replsets/tenant_migration_donor_state_machine.js +++ b/jstests/replsets/tenant_migration_donor_state_machine.js @@ -50,7 +50,7 @@ function testDonorForgetMigrationAfterMigrationCompletes( }); assert.soon(() => 0 === donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).count({ - tenantId: tenantId + _id: migrationId, })); assert.soon(() => 0 === donorPrimary.adminCommand({serverStatus: 1}) @@ -68,7 +68,7 @@ function testDonorForgetMigrationAfterMigrationCompletes( assert.soon(() => 0 === recipientPrimary.getCollection(TenantMigrationTest.kConfigRecipientsNS).count({ - tenantId: tenantId + _id: migrationId, })); assert.soon(() => 0 === recipientPrimary.adminCommand({serverStatus: 1}) @@ -149,11 +149,10 @@ function testStats(node, { assert.eq(mtab.donor.state, TenantMigrationTest.DonorAccessState.kBlockWritesAndReads); assert(mtab.donor.blockTimestamp); - let donorDoc = configDonorsColl.findOne({tenantId: kTenantId}); + let donorDoc = configDonorsColl.findOne({_id: migrationId}); let blockOplogEntry = donorPrimary.getDB("local") - .oplog.rs - .find({ns: TenantMigrationTest.kConfigDonorsNS, op: "u", "o.tenantId": kTenantId}) + .oplog.rs.find({ns: TenantMigrationTest.kConfigDonorsNS, op: "u", "o._id": migrationId}) .sort({"$natural": -1}) .limit(1) .next(); @@ -173,7 +172,7 @@ function testStats(node, { TenantMigrationTest.assertCommitted( tenantMigrationTest.waitForMigrationToComplete(migrationOpts)); - donorDoc = configDonorsColl.findOne({tenantId: kTenantId}); + donorDoc = configDonorsColl.findOne({_id: migrationId}); let commitOplogEntry = donorPrimary.getDB("local").oplog.rs.findOne( {ns: TenantMigrationTest.kConfigDonorsNS, op: "u", o: donorDoc}); assert.eq(donorDoc.state, TenantMigrationTest.DonorState.kCommitted); @@ -215,7 +214,7 @@ function testStats(node, { tenantMigrationTest.runMigration(migrationOpts, {automaticForgetMigration: false})); abortRecipientFp.off(); - const donorDoc = configDonorsColl.findOne({tenantId: kTenantId}); + const donorDoc = configDonorsColl.findOne({_id: migrationId}); const abortOplogEntry = donorPrimary.getDB("local").oplog.rs.findOne( {ns: TenantMigrationTest.kConfigDonorsNS, op: "u", o: donorDoc}); assert.eq(donorDoc.state, TenantMigrationTest.DonorState.kAborted); @@ -257,7 +256,7 @@ function testStats(node, { tenantMigrationTest.runMigration(migrationOpts, {automaticForgetMigration: false})); abortDonorFp.off(); - const donorDoc = configDonorsColl.findOne({tenantId: kTenantId}); + const donorDoc = configDonorsColl.findOne({_id: migrationId}); const abortOplogEntry = donorPrimary.getDB("local").oplog.rs.findOne( {ns: TenantMigrationTest.kConfigDonorsNS, op: "u", o: donorDoc}); assert.eq(donorDoc.state, TenantMigrationTest.DonorState.kAborted); diff --git a/jstests/replsets/tenant_migration_donor_unblock_reads_and_writes_on_completion.js b/jstests/replsets/tenant_migration_donor_unblock_reads_and_writes_on_completion.js index b1ca0dcc56c..40173bdeab8 100644 --- a/jstests/replsets/tenant_migration_donor_unblock_reads_and_writes_on_completion.js +++ b/jstests/replsets/tenant_migration_donor_unblock_reads_and_writes_on_completion.js @@ -107,7 +107,7 @@ const kCollName = "testColl"; // Run a read command against one of the secondaries, and wait for it to block. const laggedSecondary = donorRst.getSecondary(); - const donorDoc = donorsColl.findOne({tenantId: tenantId}); + const donorDoc = donorsColl.findOne({_id: migrationId}); assert.neq(null, donorDoc); const readThread = startReadThread(laggedSecondary, dbName, kCollName, donorDoc.blockTimestamp); assert.soon(() => TenantMigrationUtil.getNumBlockedReads(laggedSecondary, tenantId) == 1); @@ -155,7 +155,7 @@ const kCollName = "testColl"; // Run a read command against one of the secondaries, and wait for it to block. const laggedSecondary = donorRst.getSecondary(); - const donorDoc = donorsColl.findOne({tenantId: tenantId}); + const donorDoc = donorsColl.findOne({_id: migrationId}); assert.neq(null, donorDoc); const readThread = startReadThread(laggedSecondary, dbName, kCollName, donorDoc.blockTimestamp); assert.soon(() => TenantMigrationUtil.getNumBlockedReads(laggedSecondary, tenantId) == 1); @@ -200,7 +200,7 @@ const kCollName = "testColl"; blockingFp.wait(); // Run a read command and a write command against the primary, and wait for them to block. - const donorDoc = donorsColl.findOne({tenantId: tenantId}); + const donorDoc = donorsColl.findOne({_id: migrationId}); assert.neq(null, donorDoc); const readThread = startReadThread(donorPrimary, dbName, kCollName, donorDoc.blockTimestamp); const writeThread = startWriteThread(donorPrimary, dbName, kCollName); @@ -212,8 +212,7 @@ const kCollName = "testColl"; // Cannot mark the state doc as garbage collectable before the migration commits or aborts. assert.commandFailedWithCode( - donorsColl.update({tenantId: tenantId}, {$set: {expireAt: new Date()}}), - ErrorCodes.BadValue); + donorsColl.update({_id: migrationId}, {$set: {expireAt: new Date()}}), ErrorCodes.BadValue); // Can drop the state doc collection but this will not cause all blocked reads and writes to // hang. diff --git a/jstests/replsets/tenant_migration_drop_state_doc_collection.js b/jstests/replsets/tenant_migration_drop_state_doc_collection.js index 0eee1ca2f93..ad858d0096c 100644 --- a/jstests/replsets/tenant_migration_drop_state_doc_collection.js +++ b/jstests/replsets/tenant_migration_drop_state_doc_collection.js @@ -35,7 +35,7 @@ function makeTenantId() { function makeMigrationOpts(tenantMigrationTest, tenantId) { return { migrationIdString: extractUUIDFromObject(UUID()), - tenantId: tenantId, + tenantId, recipientConnString: tenantMigrationTest.getRecipientConnString() }; } @@ -69,7 +69,8 @@ function testDroppingStateDocCollections(tenantMigrationTest, fpName, { let fp; if (fpName) { - fp = configureFailPoint(donorPrimary, fpName, {tenantId: tenantId}); + fp = configureFailPoint( + donorPrimary, fpName, {migrationId: migrationOptsBeforeDrop.migrationIdString}); assert.commandWorked(tenantMigrationTest.startMigration(migrationOptsBeforeDrop)); fp.wait(); } else { @@ -80,7 +81,7 @@ function testDroppingStateDocCollections(tenantMigrationTest, fpName, { if (dropDonorsCollection) { assert(donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).drop()); let donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationOptsBeforeDrop.migrationIdString), }); assert.eq(donorDoc, null); @@ -99,7 +100,7 @@ function testDroppingStateDocCollections(tenantMigrationTest, fpName, { })); let recipientDoc = recipientPrimary.getCollection(TenantMigrationTest.kConfigRecipientsNS).findOne({ - tenantId: tenantId + _id: UUID(migrationOptsBeforeDrop.migrationIdString), }); assert.eq(recipientDoc, null); const currOpRecipient = assert.commandWorked( diff --git a/jstests/replsets/tenant_migration_fetch_committed_transactions_shard_merge.js b/jstests/replsets/tenant_migration_fetch_committed_transactions_shard_merge.js index df1660b66c9..7129ad46dfa 100644 --- a/jstests/replsets/tenant_migration_fetch_committed_transactions_shard_merge.js +++ b/jstests/replsets/tenant_migration_fetch_committed_transactions_shard_merge.js @@ -122,8 +122,6 @@ jsTestLog("Running a migration"); const migrationId = UUID(); const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), - // TODO(SERVER-63454): Remove tenantId when it is no longer required for shard merge. - tenantId, }; const fpAfterFetchingCommittedTransactions = diff --git a/jstests/replsets/tenant_migration_invalid_inputs.js b/jstests/replsets/tenant_migration_invalid_inputs.js index e2b5c1687ab..15548f54555 100644 --- a/jstests/replsets/tenant_migration_invalid_inputs.js +++ b/jstests/replsets/tenant_migration_invalid_inputs.js @@ -7,6 +7,7 @@ * @tags: [ * incompatible_with_macos, * incompatible_with_windows_tls, + * incompatible_with_shard_merge, * requires_persistence, * requires_fcv_51, * serverless, @@ -38,17 +39,16 @@ assert.commandFailedWithCode( donorPrimary.adminCommand( TenantMigrationUtil.donorStartMigrationWithProtocol({ donorStartMigration: 1, - protocol: 'multitenant migrations', migrationId: UUID(), recipientConnectionString: tenantMigrationTest.getRecipientRst().getURL(), - readPreference: readPreference, + readPreference, donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }, donorPrimary.getDB("admin"))), ErrorCodes.InvalidOptions); -// Test unsupported database prefixes. +// Test empty tenantId and unsupported database prefixes. const unsupportedtenantIds = ['', 'admin', 'local', 'config']; unsupportedtenantIds.forEach((invalidTenantId) => { assert.commandFailedWithCode( @@ -58,12 +58,12 @@ unsupportedtenantIds.forEach((invalidTenantId) => { migrationId: UUID(), recipientConnectionString: tenantMigrationTest.getRecipientRst().getURL(), tenantId: invalidTenantId, - readPreference: readPreference, + readPreference, donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }, donorPrimary.getDB("admin"))), - ErrorCodes.BadValue); + [ErrorCodes.InvalidOptions, ErrorCodes.BadValue]); }); // Test migrating a tenant to the donor itself. @@ -73,8 +73,8 @@ assert.commandFailedWithCode( donorStartMigration: 1, migrationId: UUID(), recipientConnectionString: tenantMigrationTest.getDonorRst().getURL(), - tenantId: tenantId, - readPreference: readPreference, + tenantId, + readPreference, donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }, @@ -89,8 +89,8 @@ assert.commandFailedWithCode( migrationId: UUID(), recipientConnectionString: tenantMigrationTest.getRecipientRst().getURL() + "," + donorPrimary.host, - tenantId: tenantId, - readPreference: readPreference, + tenantId, + readPreference, donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }, @@ -104,8 +104,8 @@ assert.commandFailedWithCode( donorStartMigration: 1, migrationId: UUID(), recipientConnectionString: recipientPrimary.host, - tenantId: tenantId, - readPreference: readPreference, + tenantId, + readPreference, donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }, @@ -117,11 +117,10 @@ jsTestLog("Testing 'recipientSyncData' command provided with invalid options."); // Test missing tenantId field for protocol 'multitenant migrations'. assert.commandFailedWithCode(recipientPrimary.adminCommand({ recipientSyncData: 1, - protocol: 'multitenant migrations', migrationId: UUID(), donorConnectionString: tenantMigrationTest.getDonorRst().getURL(), startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), ErrorCodes.InvalidOptions); @@ -134,10 +133,10 @@ unsupportedtenantIds.forEach((invalidTenantId) => { donorConnectionString: tenantMigrationTest.getDonorRst().getURL(), tenantId: invalidTenantId, startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), - ErrorCodes.BadValue); + [ErrorCodes.InvalidOptions, ErrorCodes.BadValue]); }); // Test migrating a tenant from the recipient itself. @@ -147,7 +146,7 @@ assert.commandFailedWithCode(recipientPrimary.adminCommand({ donorConnectionString: tenantMigrationTest.getRecipientRst().getURL(), tenantId: tenantId, startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), ErrorCodes.BadValue); @@ -156,10 +155,10 @@ assert.commandFailedWithCode(recipientPrimary.adminCommand({ assert.commandFailedWithCode(recipientPrimary.adminCommand({ recipientSyncData: 1, migrationId: UUID(), - donorConnectionString: tenantMigrationTest.getDonorRst().getURL() + "," + recipientPrimary.host, + donorConnectionString: `${tenantMigrationTest.getDonorRst().getURL()},${recipientPrimary.host}`, tenantId: tenantId, startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), ErrorCodes.BadValue); @@ -171,7 +170,7 @@ assert.commandFailedWithCode(recipientPrimary.adminCommand({ donorConnectionString: recipientPrimary.host, tenantId: tenantId, startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), ErrorCodes.BadValue); @@ -185,7 +184,7 @@ nullTimestamps.forEach((nullTs) => { donorConnectionString: tenantMigrationTest.getDonorRst().getURL(), tenantId: tenantId, startMigrationDonorTimestamp: Timestamp(1, 1), - readPreference: readPreference, + readPreference, returnAfterReachingDonorTimestamp: nullTs, recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, }), diff --git a/jstests/replsets/tenant_migration_recipient_aborts_merge_on_donor_failure.js b/jstests/replsets/tenant_migration_recipient_aborts_merge_on_donor_failure.js index 858c322f64d..4b9b85f905d 100644 --- a/jstests/replsets/tenant_migration_recipient_aborts_merge_on_donor_failure.js +++ b/jstests/replsets/tenant_migration_recipient_aborts_merge_on_donor_failure.js @@ -4,6 +4,7 @@ * @tags: [ * incompatible_with_macos, * incompatible_with_windows_tls, + * featureFlagShardMerge, * requires_majority_read_concern, * requires_persistence, * serverless, @@ -47,7 +48,6 @@ load("jstests/replsets/libs/tenant_migration_util.js"); const migrationUuid = UUID(); const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationUuid), - tenantId, readPreference: {mode: 'primary'} }; diff --git a/jstests/replsets/tenant_migration_recipient_current_op.js b/jstests/replsets/tenant_migration_recipient_current_op.js index 19d1bbdbd6d..0ca466c7a27 100644 --- a/jstests/replsets/tenant_migration_recipient_current_op.js +++ b/jstests/replsets/tenant_migration_recipient_current_op.js @@ -59,13 +59,21 @@ for (const db of dbsToClone) { // correct. function checkStandardFieldsOK(res) { assert.eq(res.inprog.length, 1, res); - assert.eq(bsonWoCompare(res.inprog[0].instanceID, kMigrationId), 0, res); - assert.eq(res.inprog[0].donorConnectionString, tenantMigrationTest.getDonorRst().getURL(), res); - assert.eq(bsonWoCompare(res.inprog[0].readPreference, kReadPreference), 0, res); + const { + instanceID, + donorConnectionString, + readPreference, + numRestartsDueToDonorConnectionFailure, + numRestartsDueToRecipientFailure, + tenantId + } = res.inprog[0]; + assert.eq(bsonWoCompare(instanceID, kMigrationId), 0, res); + assert.eq(donorConnectionString, tenantMigrationTest.getDonorRst().getURL(), res); + assert.eq(bsonWoCompare(readPreference, kReadPreference), 0, res); // We don't test failovers in this test so we don't expect these counters to be incremented. - assert.eq(res.inprog[0].numRestartsDueToDonorConnectionFailure, 0, res); - assert.eq(res.inprog[0].numRestartsDueToRecipientFailure, 0, res); - assert.eq(bsonWoCompare(res.inprog[0].tenantId, kTenantId), 0, res); + assert.eq(numRestartsDueToDonorConnectionFailure, 0, res); + assert.eq(numRestartsDueToRecipientFailure, 0, res); + assert.eq(bsonWoCompare(tenantId, kTenantId), 0, res); } // Check currentOp fields' expected value once the recipient is in state "consistent" or later. diff --git a/jstests/replsets/tenant_migration_recipient_rollback_recovery.js b/jstests/replsets/tenant_migration_recipient_rollback_recovery.js index 32c562c4779..02a707d9443 100644 --- a/jstests/replsets/tenant_migration_recipient_rollback_recovery.js +++ b/jstests/replsets/tenant_migration_recipient_rollback_recovery.js @@ -143,12 +143,13 @@ function testRollbackInitialState() { let steadyStateFunc = (tenantMigrationTest) => { // Verify that the migration restarted successfully on the new primary despite rollback. TenantMigrationTest.assertCommitted(migrationThread.returnData()); - tenantMigrationTest.assertRecipientNodesInExpectedState( - tenantMigrationTest.getRecipientRst().nodes, - migrationId, - migrationOpts.tenantId, - TenantMigrationTest.RecipientState.kConsistent, - TenantMigrationTest.RecipientAccessState.kRejectBefore); + tenantMigrationTest.assertRecipientNodesInExpectedState({ + nodes: tenantMigrationTest.getRecipientRst().nodes, + migrationId: migrationId, + tenantId: migrationOpts.tenantId, + expectedState: TenantMigrationTest.RecipientState.kConsistent, + expectedAccessState: TenantMigrationTest.RecipientAccessState.kRejectBefore + }); assert.commandWorked(tenantMigrationTest.forgetMigration(migrationOpts.migrationIdString)); }; diff --git a/jstests/replsets/tenant_migration_recipient_shard_merge_learn_files.js b/jstests/replsets/tenant_migration_recipient_shard_merge_learn_files.js index e6dc611f85c..9fe0b9adea5 100644 --- a/jstests/replsets/tenant_migration_recipient_shard_merge_learn_files.js +++ b/jstests/replsets/tenant_migration_recipient_shard_merge_learn_files.js @@ -38,14 +38,9 @@ tenantMigrationTest.insertDonorDB(tenantDB, collName); const failpoint = "fpBeforeMarkingCloneSuccess"; const waitInFailPoint = configureFailPoint(recipientPrimary, failpoint, {action: "hang"}); -// In order to prevent the copying of "testTenantId" databases via logical cloning from donor to -// recipient, start migration on a tenant id which is non-existent on the donor. const migrationUuid = UUID(); -const kDummyTenantId = "nonExistentTenantId"; const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationUuid), - // TODO (SERVER-63454): Remove kDummyTenantId. - tenantId: kDummyTenantId, readPreference: {mode: 'primary'} }; @@ -55,12 +50,13 @@ assert.commandWorked( waitInFailPoint.wait(); -tenantMigrationTest.assertRecipientNodesInExpectedState( - tenantMigrationTest.getRecipientRst().nodes, - migrationUuid, +tenantMigrationTest.assertRecipientNodesInExpectedState({ + nodes: tenantMigrationTest.getRecipientRst().nodes, + migrationId: migrationUuid, tenantId, - TenantMigrationTest.RecipientState.kLearnedFilenames, - TenantMigrationTest.RecipientAccessState.kReject); + expectedState: TenantMigrationTest.RecipientState.kLearnedFilenames, + expectedAccessState: TenantMigrationTest.RecipientAccessState.kReject +}); waitInFailPoint.off(); diff --git a/jstests/replsets/tenant_migration_recipient_shard_merge_oplog_catchup.js b/jstests/replsets/tenant_migration_recipient_shard_merge_oplog_catchup.js index a2b8008bcb1..8cb4bbf41c2 100644 --- a/jstests/replsets/tenant_migration_recipient_shard_merge_oplog_catchup.js +++ b/jstests/replsets/tenant_migration_recipient_shard_merge_oplog_catchup.js @@ -33,13 +33,9 @@ const failpoint = "pauseTenantMigrationBeforeLeavingDataSyncState"; const pauseTenantMigrationBeforeLeavingDataSyncState = configureFailPoint(donorPrimary, failpoint, {action: "hang"}); -// Start migration on a tenant id which is non-existent on the donor. const migrationUuid = UUID(); -const kDummyTenantId = "nonExistentTenantId"; const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationUuid), - // TODO (SERVER-63454): Remove kDummyTenantId. - tenantId: kDummyTenantId, readPreference: {mode: 'primary'} }; diff --git a/jstests/replsets/tenant_migration_recipient_vote_imported_files.js b/jstests/replsets/tenant_migration_recipient_vote_imported_files.js index b1a489155b1..d5d5138bcf6 100644 --- a/jstests/replsets/tenant_migration_recipient_vote_imported_files.js +++ b/jstests/replsets/tenant_migration_recipient_vote_imported_files.js @@ -57,7 +57,6 @@ const migrationId = UUID(); const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), recipientConnString: tenantMigrationTest.getRecipientConnString(), - tenantId: kTenantId, }; const donorRstArgs = TenantMigrationUtil.createRstArgs(tenantMigrationTest.getDonorRst()); diff --git a/jstests/replsets/tenant_migration_shard_merge_import_write_conflict_retry.js b/jstests/replsets/tenant_migration_shard_merge_import_write_conflict_retry.js index cded13b4ddf..11c8d1d2816 100644 --- a/jstests/replsets/tenant_migration_shard_merge_import_write_conflict_retry.js +++ b/jstests/replsets/tenant_migration_shard_merge_import_write_conflict_retry.js @@ -66,10 +66,8 @@ configureFailPoint( jsTestLog("Run migration"); // The old multitenant migrations won't copy myDatabase since it doesn't start with testTenantId, // but shard merge copies everything so we still expect myDatabase on the recipient, below. -const kTenantId = "testTenantId"; const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), - tenantId: kTenantId, }; TenantMigrationTest.assertCommitted( tenantMigrationTest.runMigration(migrationOpts, {enableDonorStartMigrationFsync: true})); diff --git a/jstests/replsets/tenant_migration_shard_merge_invalid_inputs.js b/jstests/replsets/tenant_migration_shard_merge_invalid_inputs.js new file mode 100644 index 00000000000..b4c74dfd1cd --- /dev/null +++ b/jstests/replsets/tenant_migration_shard_merge_invalid_inputs.js @@ -0,0 +1,162 @@ +/** + * Tests that the donorStartMigration and recipientSyncData commands for a shard merge throw an + * error if a tenantId is provided or if the prefix is invalid (i.e. '', 'admin', 'local' or + * 'config') or if the recipient connection string matches the donor's connection string or doesn't + * correspond to a replica set with a least one host. + * + * @tags: [ + * incompatible_with_eft, + * incompatible_with_macos, + * incompatible_with_windows_tls, + * featureFlagShardMerge, + * requires_persistence, + * requires_fcv_51, + * serverless, + * ] + */ + +(function() { +"use strict"; + +load("jstests/replsets/libs/tenant_migration_test.js"); +load("jstests/replsets/libs/tenant_migration_util.js"); + +const tenantMigrationTest = + new TenantMigrationTest({name: jsTestName(), enableRecipientTesting: false}); + +const donorPrimary = tenantMigrationTest.getDonorPrimary(); +const recipientPrimary = tenantMigrationTest.getRecipientPrimary(); + +const tenantId = "testTenantId"; +const readPreference = { + mode: 'primary' +}; +const migrationCertificates = TenantMigrationUtil.makeMigrationCertificatesForTest(); + +jsTestLog("Testing 'donorStartMigration' command provided with invalid options."); + +// Test erroneously included tenantId field and unsupported database prefixes. +const unsupportedtenantIds = [tenantId, 'admin', 'local', 'config']; +unsupportedtenantIds.forEach((invalidTenantId) => { + const cmd = { + donorStartMigration: 1, + migrationId: UUID(), + protocol: 'shard merge', + tenantId: invalidTenantId, + recipientConnectionString: tenantMigrationTest.getRecipientRst().getURL(), + readPreference, + donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, + }; + assert.commandFailedWithCode(donorPrimary.adminCommand(cmd), + [ErrorCodes.InvalidOptions, ErrorCodes.BadValue]); +}); + +// Test merging to the donor itself. +assert.commandFailedWithCode(donorPrimary.adminCommand({ + donorStartMigration: 1, + migrationId: UUID(), + protocol: 'shard merge', + recipientConnectionString: tenantMigrationTest.getDonorRst().getURL(), + readPreference, + donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +// Test merging to a recipient that shares one or more hosts with the donor. +assert.commandFailedWithCode(donorPrimary.adminCommand({ + donorStartMigration: 1, + migrationId: UUID(), + protocol: 'shard merge', + recipientConnectionString: + tenantMigrationTest.getRecipientRst().getURL() + "," + donorPrimary.host, + readPreference, + donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +// Test merging to a standalone recipient. +assert.commandFailedWithCode(donorPrimary.adminCommand({ + donorStartMigration: 1, + migrationId: UUID(), + protocol: 'shard merge', + recipientConnectionString: recipientPrimary.host, + readPreference, + donorCertificateForRecipient: migrationCertificates.donorCertificateForRecipient, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +jsTestLog("Testing 'recipientSyncData' command provided with invalid options."); + +// Test erroneously included tenantId field and unsupported database prefixes. +unsupportedtenantIds.forEach((invalidTenantId) => { + assert.commandFailedWithCode(recipientPrimary.adminCommand({ + recipientSyncData: 1, + migrationId: UUID(), + donorConnectionString: tenantMigrationTest.getDonorRst().getURL(), + tenantId: invalidTenantId, + protocol: 'shard merge', + startMigrationDonorTimestamp: Timestamp(1, 1), + readPreference, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, + }), + [ErrorCodes.InvalidOptions, ErrorCodes.BadValue]); +}); + +// Test merging from the recipient itself. +assert.commandFailedWithCode(recipientPrimary.adminCommand({ + recipientSyncData: 1, + migrationId: UUID(), + protocol: 'shard merge', + donorConnectionString: tenantMigrationTest.getRecipientRst().getURL(), + startMigrationDonorTimestamp: Timestamp(1, 1), + readPreference, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +// Test merging from a donor that shares one or more hosts with the recipient. +assert.commandFailedWithCode(recipientPrimary.adminCommand({ + recipientSyncData: 1, + migrationId: UUID(), + protocol: 'shard merge', + donorConnectionString: `${tenantMigrationTest.getDonorRst().getURL()},${recipientPrimary.host}`, + startMigrationDonorTimestamp: Timestamp(1, 1), + readPreference, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +// Test merging from a standalone donor. +assert.commandFailedWithCode(recipientPrimary.adminCommand({ + recipientSyncData: 1, + migrationId: UUID(), + protocol: 'shard merge', + donorConnectionString: recipientPrimary.host, + startMigrationDonorTimestamp: Timestamp(1, 1), + readPreference, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, +}), + ErrorCodes.BadValue); + +// Test 'returnAfterReachingDonorTimestamp' can't be null. +const nullTimestamps = [Timestamp(0, 0), Timestamp(0, 1)]; +nullTimestamps.forEach((nullTs) => { + assert.commandFailedWithCode(donorPrimary.adminCommand({ + recipientSyncData: 1, + migrationId: UUID(), + protocol: 'shard merge', + donorConnectionString: tenantMigrationTest.getDonorRst().getURL(), + startMigrationDonorTimestamp: Timestamp(1, 1), + readPreference, + returnAfterReachingDonorTimestamp: nullTs, + recipientCertificateForDonor: migrationCertificates.recipientCertificateForDonor, + }), + ErrorCodes.BadValue); +}); + +tenantMigrationTest.stop(); +})(); diff --git a/jstests/replsets/tenant_migration_shard_merge_recipient_creates_blockers_during_migration.js b/jstests/replsets/tenant_migration_shard_merge_recipient_creates_blockers_during_migration.js index 7cdd2ed9deb..ed375658f20 100644 --- a/jstests/replsets/tenant_migration_shard_merge_recipient_creates_blockers_during_migration.js +++ b/jstests/replsets/tenant_migration_shard_merge_recipient_creates_blockers_during_migration.js @@ -65,8 +65,6 @@ const waitInFailPoint = configureFailPoint(recipientPrimary, failpoint, {action: const migrationId = UUID(); const migrationOpts = { migrationIdString: extractUUIDFromObject(migrationId), - // TODO (SERVER-63454): Remove tenantId. - tenantId, }; assert.commandWorked(tenantMigrationTest.startMigration(migrationOpts)); diff --git a/jstests/replsets/tenant_migration_shard_merge_recipient_current_op.js b/jstests/replsets/tenant_migration_shard_merge_recipient_current_op.js index 977a0340534..dbb0717ae03 100644 --- a/jstests/replsets/tenant_migration_shard_merge_recipient_current_op.js +++ b/jstests/replsets/tenant_migration_shard_merge_recipient_current_op.js @@ -33,7 +33,6 @@ const kReadPreference = { }; const migrationOpts = { migrationIdString: extractUUIDFromObject(kMigrationId), - tenantId: kTenantId, readPreference: kReadPreference }; @@ -60,7 +59,6 @@ function checkStandardFieldsOK(res) { // We don't test failovers in this test so we don't expect these counters to be incremented. assert.eq(res.inprog[0].numRestartsDueToDonorConnectionFailure, 0, res); assert.eq(res.inprog[0].numRestartsDueToRecipientFailure, 0, res); - assert.eq(bsonWoCompare(res.inprog[0].tenantId, kTenantId), 0, res); } // Check currentOp fields' expected value once the recipient is in state "consistent" or later. @@ -101,8 +99,7 @@ const fpAfterDataConsistent = configureFailPoint( const fpAfterForgetMigration = configureFailPoint( recipientPrimary, "fpAfterReceivingRecipientForgetMigration", {action: "hang"}); -jsTestLog("Starting tenant migration with migrationId: " + kMigrationId + - ", tenantId: " + kTenantId); +jsTestLog(`Starting tenant migration with migrationId: ${kMigrationId}`); assert.commandWorked( tenantMigrationTest.startMigration(migrationOpts, {enableDonorStartMigrationFsync: true})); diff --git a/jstests/replsets/tenant_migration_timeseries_retryable_write_oplog_cloning.js b/jstests/replsets/tenant_migration_timeseries_retryable_write_oplog_cloning.js index 2a62d8e5fb5..bd5759a35ca 100644 --- a/jstests/replsets/tenant_migration_timeseries_retryable_write_oplog_cloning.js +++ b/jstests/replsets/tenant_migration_timeseries_retryable_write_oplog_cloning.js @@ -118,9 +118,8 @@ function testOplogCloning(ordered) { TenantMigrationTest.assertCommitted( tenantMigrationTest.runMigration(migrationOpts, {automaticForgetMigration: false})); - const donorDoc = donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({ - tenantId: kTenantId - }); + const donorDoc = + donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({_id: migrationId}); assert.commandWorked(tenantMigrationTest.forgetMigration(migrationOpts.migrationIdString)); tenantMigrationTest.waitForMigrationGarbageCollection(migrationId, kTenantId); diff --git a/jstests/replsets/tenant_migrations_back_to_back.js b/jstests/replsets/tenant_migrations_back_to_back.js index 11c19515923..4aaa6682de8 100644 --- a/jstests/replsets/tenant_migrations_back_to_back.js +++ b/jstests/replsets/tenant_migrations_back_to_back.js @@ -53,7 +53,7 @@ migrationThread.start(); waitForRejectReadsBeforeTsFp.wait(); const donorDoc = - donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({tenantId: kTenantId}); + donorPrimary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({_id: migrationId}); assert.lt(preMigrationTimestamp, donorDoc.blockTimestamp); waitForRejectReadsBeforeTsFp.off(); // Wait for the migration to complete. @@ -115,7 +115,7 @@ assert.eq(res.inprog[0].lastDurableState, kBlocking, tojson(res.inprog)); // Get the block timestamp for this new migration. const donorDoc2 = - donor2Primary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({tenantId: kTenantId}); + donor2Primary.getCollection(TenantMigrationTest.kConfigDonorsNS).findOne({_id: migration2Id}); assert.eq( mtabStatus.donor.state, TenantMigrationTest.DonorAccessState.kBlockWritesAndReads, mtabStatus); assert(mtabStatus.donor.hasOwnProperty("blockTimestamp"), mtabStatus); diff --git a/src/mongo/db/commands/tenant_migration_donor_cmds.cpp b/src/mongo/db/commands/tenant_migration_donor_cmds.cpp index 0be8a2da13a..0a87812b0f4 100644 --- a/src/mongo/db/commands/tenant_migration_donor_cmds.cpp +++ b/src/mongo/db/commands/tenant_migration_donor_cmds.cpp @@ -76,8 +76,8 @@ public: const auto& cmd = request(); const auto migrationProtocol = cmd.getProtocol().value_or(kDefaultMigrationProtocol); - uassertStatusOK(tenant_migration_util::protocolTenantIdCompatibilityCheck( - migrationProtocol, cmd.getTenantId().toString())); + tenant_migration_util::protocolTenantIdCompatibilityCheck(migrationProtocol, + cmd.getTenantId().toString()); tenant_migration_util::protocolStorageOptionsCompatibilityCheck(opCtx, migrationProtocol); diff --git a/src/mongo/db/commands/tenant_migration_donor_cmds.idl b/src/mongo/db/commands/tenant_migration_donor_cmds.idl index 3117ef7a9ba..bd6a946e360 100644 --- a/src/mongo/db/commands/tenant_migration_donor_cmds.idl +++ b/src/mongo/db/commands/tenant_migration_donor_cmds.idl @@ -69,7 +69,7 @@ commands: validator: callback: "tenant_migration_util::validateConnectionString" tenantId: - description: "The prefix from which the migrating database will be matched. The prefixes 'admin', 'local', 'config', the empty string, are not allowed." + description: "The prefix from which the migrating database will be matched. The prefixes 'admin', 'local', and 'config' are not allowed." type: string default: '""' validator: diff --git a/src/mongo/db/commands/tenant_migration_recipient_cmds.cpp b/src/mongo/db/commands/tenant_migration_recipient_cmds.cpp index 3ea00d63736..8bc70b666ad 100644 --- a/src/mongo/db/commands/tenant_migration_recipient_cmds.cpp +++ b/src/mongo/db/commands/tenant_migration_recipient_cmds.cpp @@ -75,8 +75,8 @@ public: const auto& cmd = request(); const auto migrationProtocol = cmd.getProtocol().value_or(kDefaultMigrationProtocol); - uassertStatusOK(tenant_migration_util::protocolTenantIdCompatibilityCheck( - migrationProtocol, cmd.getTenantId().toString())); + tenant_migration_util::protocolTenantIdCompatibilityCheck(migrationProtocol, + cmd.getTenantId().toString()); tenant_migration_util::protocolStorageOptionsCompatibilityCheck(opCtx, migrationProtocol); @@ -244,8 +244,8 @@ public: const auto& cmd = request(); const auto migrationProtocol = cmd.getProtocol().value_or(kDefaultMigrationProtocol); - uassertStatusOK(tenant_migration_util::protocolTenantIdCompatibilityCheck( - migrationProtocol, cmd.getTenantId().toString())); + tenant_migration_util::protocolTenantIdCompatibilityCheck(migrationProtocol, + cmd.getTenantId().toString()); opCtx->setAlwaysInterruptAtStepDownOrUp_UNSAFE(); auto recipientService = diff --git a/src/mongo/db/commands/tenant_migration_recipient_cmds.idl b/src/mongo/db/commands/tenant_migration_recipient_cmds.idl index b3495f02d12..29643aded8e 100644 --- a/src/mongo/db/commands/tenant_migration_recipient_cmds.idl +++ b/src/mongo/db/commands/tenant_migration_recipient_cmds.idl @@ -68,7 +68,7 @@ structs: tenantId: description: >- The prefix from which the migrating database will be matched. The prefixes 'admin', - 'local', 'config', the empty string, are not allowed. + 'local', 'config', and the empty string are not allowed. type: string default: '""' validator: diff --git a/src/mongo/db/repl/tenant_migration_access_blocker_registry.cpp b/src/mongo/db/repl/tenant_migration_access_blocker_registry.cpp index cb00203ea33..6f0fde3e5e6 100644 --- a/src/mongo/db/repl/tenant_migration_access_blocker_registry.cpp +++ b/src/mongo/db/repl/tenant_migration_access_blocker_registry.cpp @@ -163,21 +163,35 @@ void TenantMigrationAccessBlockerRegistry::removeShardMergeDonorAccessBlocker( } } -void TenantMigrationAccessBlockerRegistry::removeRecipientAccessBlockersForMigration( - const UUID& migrationId) { +void TenantMigrationAccessBlockerRegistry::removeAccessBlockersForMigration( + const UUID& migrationId, TenantMigrationAccessBlocker::BlockerType type) { + if (type == MtabType::kDonor) { + removeShardMergeDonorAccessBlocker(migrationId); + } + stdx::lock_guard<Latch> lg(_mutex); - // Clear recipient blockers for migrationId, and erase pairs with no blocker remaining. + // Clear blockers for migrationId, and erase pairs with no blocker remaining. erase_if(_tenantMigrationAccessBlockers, [&](std::pair<const std::string, DonorRecipientAccessBlockerPair> it) { auto& mtabPair = it.second; - auto recipient = checked_pointer_cast<TenantMigrationRecipientAccessBlocker>( - mtabPair.getAccessBlocker(MtabType::kRecipient)); - if (!recipient || recipient->getMigrationId() != migrationId) { + auto blocker = mtabPair.getAccessBlocker(type); + if (!blocker || blocker->getMigrationId() != migrationId) { return false; } - mtabPair.clearAccessBlocker(MtabType::kRecipient); - return !mtabPair.getAccessBlocker(MtabType::kDonor); + mtabPair.clearAccessBlocker(type); + MtabType oppositeType; + switch (type) { + case MtabType::kRecipient: + oppositeType = MtabType::kDonor; + break; + case MtabType::kDonor: + oppositeType = MtabType::kRecipient; + break; + default: + MONGO_UNREACHABLE; + } + return !mtabPair.getAccessBlocker(oppositeType); }); } @@ -253,13 +267,12 @@ TenantMigrationAccessBlockerRegistry::getTenantMigrationAccessBlockerForTenantId } } -void TenantMigrationAccessBlockerRegistry::applyAll( - TenantMigrationAccessBlocker::BlockerType type, - const std::function<void(std::shared_ptr<TenantMigrationAccessBlocker>)>& callback) { +void TenantMigrationAccessBlockerRegistry::applyAll(TenantMigrationAccessBlocker::BlockerType type, + applyAllCallback&& callback) { stdx::lock_guard<Latch> lg(_mutex); for (auto& [tenantId, mtabPair] : _tenantMigrationAccessBlockers) { if (auto mtab = mtabPair.getAccessBlocker(type)) { - callback(mtab); + callback(tenantId, mtab); } } } diff --git a/src/mongo/db/repl/tenant_migration_access_blocker_registry.h b/src/mongo/db/repl/tenant_migration_access_blocker_registry.h index 2e604c86c89..0c8b162d1bf 100644 --- a/src/mongo/db/repl/tenant_migration_access_blocker_registry.h +++ b/src/mongo/db/repl/tenant_migration_access_blocker_registry.h @@ -104,7 +104,7 @@ public: void addShardMergeDonorAccessBlocker(std::shared_ptr<TenantMigrationDonorAccessBlocker> mtab); /** - * Invariants that an entry for tenantId exists, and then removes the entry for (tenantId, mtab) + * Removes the entry for (tenantId, mtab) */ void remove(StringData tenantId, TenantMigrationAccessBlocker::BlockerType type); @@ -115,9 +115,10 @@ public: void removeShardMergeDonorAccessBlocker(const UUID& migrationId); /** - * Remove all recipient access blockers for a migration. + * Remove all access blockers of the provided type for a migration. */ - void removeRecipientAccessBlockersForMigration(const UUID& migrationId); + void removeAccessBlockersForMigration(const UUID& migrationId, + TenantMigrationAccessBlocker::BlockerType type); /** * Removes all mtabs of the given type. @@ -147,12 +148,12 @@ public: std::shared_ptr<TenantMigrationAccessBlocker> getTenantMigrationAccessBlockerForTenantId( StringData tenantId, TenantMigrationAccessBlocker::BlockerType type); + using applyAllCallback = std::function<void( + std::string tenantId, std::shared_ptr<TenantMigrationAccessBlocker>& mtab)>; /** * Applies callback to all TenantMigrationAccessBlockers of the desired type. */ - void applyAll( - TenantMigrationAccessBlocker::BlockerType type, - const std::function<void(std::shared_ptr<TenantMigrationAccessBlocker>)>& callback); + void applyAll(TenantMigrationAccessBlocker::BlockerType type, applyAllCallback&& callback); /** * Shuts down each of the TenantMigrationAccessBlockers and releases the shared_ptrs to the diff --git a/src/mongo/db/repl/tenant_migration_access_blocker_util.cpp b/src/mongo/db/repl/tenant_migration_access_blocker_util.cpp index 19b9a705b20..ed285df2043 100644 --- a/src/mongo/db/repl/tenant_migration_access_blocker_util.cpp +++ b/src/mongo/db/repl/tenant_migration_access_blocker_util.cpp @@ -86,10 +86,8 @@ std::shared_ptr<TenantMigrationRecipientAccessBlocker> getTenantMigrationRecipie .getTenantMigrationAccessBlockerForTenantId(tenantId, MtabType::kRecipient)); } -void startRejectingReadsBefore(OperationContext* opCtx, - const UUID& migrationId, - mongo::Timestamp ts) { - auto callback = [&](std::shared_ptr<TenantMigrationAccessBlocker> mtab) { +void startRejectingReadsBefore(OperationContext* opCtx, mongo::Timestamp ts) { + auto callback = [&](std::string _, std::shared_ptr<TenantMigrationAccessBlocker>& mtab) { auto recipientMtab = checked_pointer_cast<TenantMigrationRecipientAccessBlocker>(mtab); recipientMtab->startRejectingReadsBefore(ts); }; @@ -347,11 +345,11 @@ void recoverTenantMigrationAccessBlockers(OperationContext* opCtx) { return true; } - auto protocol = doc.getProtocol().value_or(MigrationProtocolEnum::kMultitenantMigrations); auto mtab = std::make_shared<TenantMigrationDonorAccessBlocker>(opCtx->getServiceContext(), doc.getId()); auto& registry = TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()); + auto protocol = doc.getProtocol().value_or(MigrationProtocolEnum::kMultitenantMigrations); if (protocol == MigrationProtocolEnum::kMultitenantMigrations) { registry.add(doc.getTenantId(), mtab); } else { @@ -400,6 +398,11 @@ void recoverTenantMigrationAccessBlockers(OperationContext* opCtx) { return true; } + auto protocol = doc.getProtocol().value_or(MigrationProtocolEnum::kMultitenantMigrations); + if (protocol == MigrationProtocolEnum::kShardMerge) { + return true; + } + auto mtab = std::make_shared<TenantMigrationRecipientAccessBlocker>( opCtx->getServiceContext(), doc.getId()); @@ -414,12 +417,13 @@ void recoverTenantMigrationAccessBlockers(OperationContext* opCtx) { case TenantMigrationRecipientStateEnum::kConsistent: case TenantMigrationRecipientStateEnum::kDone: if (doc.getRejectReadsBeforeTimestamp()) { - mtab->startRejectingReadsBefore(doc.getRejectReadsBeforeTimestamp().value()); + mtab->startRejectingReadsBefore(doc.getRejectReadsBeforeTimestamp().get()); } break; case TenantMigrationRecipientStateEnum::kUninitialized: MONGO_UNREACHABLE; } + return true; }); diff --git a/src/mongo/db/repl/tenant_migration_access_blocker_util.h b/src/mongo/db/repl/tenant_migration_access_blocker_util.h index 78d8a19a56a..ebb52e9491f 100644 --- a/src/mongo/db/repl/tenant_migration_access_blocker_util.h +++ b/src/mongo/db/repl/tenant_migration_access_blocker_util.h @@ -49,9 +49,7 @@ std::shared_ptr<TenantMigrationRecipientAccessBlocker> getTenantMigrationRecipie /** * For "shard merge" protocol: tell all recipient access blockers to reject reads before ts. */ -void startRejectingReadsBefore(OperationContext* opCtx, - const UUID& migrationId, - mongo::Timestamp ts); +void startRejectingReadsBefore(OperationContext* opCtx, mongo::Timestamp ts); /** * Add an access blocker if one does not already exist. diff --git a/src/mongo/db/repl/tenant_migration_decoration.h b/src/mongo/db/repl/tenant_migration_decoration.h index c2018e4965d..bbc61a0046d 100644 --- a/src/mongo/db/repl/tenant_migration_decoration.h +++ b/src/mongo/db/repl/tenant_migration_decoration.h @@ -38,11 +38,8 @@ namespace mongo { namespace repl { struct TenantMigrationInfo { - TenantMigrationInfo(const UUID& in_uuid, - const boost::optional<std::string>& tenantId = boost::none) - : uuid(in_uuid), tenantId(tenantId) {} + TenantMigrationInfo(const UUID& in_uuid) : uuid(in_uuid) {} UUID uuid; - boost::optional<std::string> tenantId; }; extern const OperationContext::Decoration<boost::optional<TenantMigrationInfo>> tenantMigrationInfo; diff --git a/src/mongo/db/repl/tenant_migration_donor_op_observer.cpp b/src/mongo/db/repl/tenant_migration_donor_op_observer.cpp index 2050371b563..bc3b45c0b13 100644 --- a/src/mongo/db/repl/tenant_migration_donor_op_observer.cpp +++ b/src/mongo/db/repl/tenant_migration_donor_op_observer.cpp @@ -300,24 +300,10 @@ void TenantMigrationDonorOpObserver::aboutToDelete(OperationContext* opCtx, // TenantMigrationDonorAccessBlocker as soon as its donor state doc is marked as garbage // collectable. So onDelete should skip removing the TenantMigrationDonorAccessBlocker for // aborted migrations. - auto constructTenantMigrationInfo = [&]() -> boost::optional<TenantMigrationInfo> { - if (donorStateDoc.getProtocol().value_or( - MigrationProtocolEnum::kMultitenantMigrations) == - MigrationProtocolEnum::kMultitenantMigrations) { - return boost::make_optional(TenantMigrationInfo( - donorStateDoc.getId(), donorStateDoc.getTenantId().toString())); - } - - tassert(6448700, - "Bad protocol", - donorStateDoc.getProtocol() == MigrationProtocolEnum::kShardMerge); - return boost::make_optional(TenantMigrationInfo(donorStateDoc.getId())); - }; - tenantMigrationInfo(opCtx) = donorStateDoc.getState() == TenantMigrationDonorStateEnum::kAborted ? boost::none - : constructTenantMigrationInfo(); + : boost::make_optional(TenantMigrationInfo(donorStateDoc.getId())); } } @@ -333,25 +319,15 @@ void TenantMigrationDonorOpObserver::onDelete(OperationContext* opCtx, return; } - if (tmi->tenantId) { - auto tenantId = tmi->tenantId.get(); - LOGV2_INFO(6461600, - "Removing expired 'multitenant migration' migration", - "tenantId"_attr = tenantId); - opCtx->recoveryUnit()->onCommit([opCtx, tenantId](boost::optional<Timestamp>) { - TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) - .remove(tenantId, TenantMigrationAccessBlocker::BlockerType::kDonor); - }); - } else { - auto migrationId = tmi->uuid; + auto migrationId = tmi->uuid; + opCtx->recoveryUnit()->onCommit([opCtx, migrationId](boost::optional<Timestamp>) { LOGV2_INFO(6461601, - "Removing expired 'shard merge' migration", + "Removing expired migration access blocker", "migrationId"_attr = migrationId); - opCtx->recoveryUnit()->onCommit([opCtx, migrationId](boost::optional<Timestamp>) { - TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) - .removeShardMergeDonorAccessBlocker(migrationId); - }); - } + TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) + .removeAccessBlockersForMigration( + migrationId, TenantMigrationAccessBlocker::BlockerType::kDonor); + }); } } diff --git a/src/mongo/db/repl/tenant_migration_donor_service.cpp b/src/mongo/db/repl/tenant_migration_donor_service.cpp index 98904d323c1..20040c335a4 100644 --- a/src/mongo/db/repl/tenant_migration_donor_service.cpp +++ b/src/mongo/db/repl/tenant_migration_donor_service.cpp @@ -787,8 +787,9 @@ ExecutorFuture<void> TenantMigrationDonorService::Instance::_sendRecipientSyncDa MigrationRecipientCommonData commonData( _migrationUuid, donorConnString.toString(), _readPreference); commonData.setRecipientCertificateForDonor(_recipientCertificateForDonor); - // TODO SERVER-63454: Pass tenantId only for 'kMultitenantMigrations' protocol. - commonData.setTenantId(_tenantId); + if (_protocol == MigrationProtocolEnum::kMultitenantMigrations) { + commonData.setTenantId(_tenantId); + } stdx::lock_guard<Latch> lg(_mutex); @@ -818,8 +819,9 @@ ExecutorFuture<void> TenantMigrationDonorService::Instance::_sendRecipientForget MigrationRecipientCommonData commonData( _migrationUuid, donorConnString.toString(), _readPreference); commonData.setRecipientCertificateForDonor(_recipientCertificateForDonor); - // TODO SERVER-63454: Pass tenantId only for 'kMultitenantMigrations' protocol. - commonData.setTenantId(_tenantId); + if (_protocol == MigrationProtocolEnum::kMultitenantMigrations) { + commonData.setTenantId(_tenantId); + } commonData.setProtocol(_protocol); request.setMigrationRecipientCommonData(commonData); diff --git a/src/mongo/db/repl/tenant_migration_recipient_op_observer.cpp b/src/mongo/db/repl/tenant_migration_recipient_op_observer.cpp index 109a0365821..cc1dab4f37a 100644 --- a/src/mongo/db/repl/tenant_migration_recipient_op_observer.cpp +++ b/src/mongo/db/repl/tenant_migration_recipient_op_observer.cpp @@ -65,9 +65,58 @@ void onSetRejectReadsBeforeTimestamp(OperationContext* opCtx, mtab->startRejectingReadsBefore(recipientStateDoc.getRejectReadsBeforeTimestamp().value()); } else { tenant_migration_access_blocker::startRejectingReadsBefore( - opCtx, - recipientStateDoc.getId(), - recipientStateDoc.getRejectReadsBeforeTimestamp().value()); + opCtx, recipientStateDoc.getRejectReadsBeforeTimestamp().get()); + } +} + +void handleMTMStateChange(OperationContext* opCtx, + const TenantMigrationRecipientDocument& recipientStateDoc) { + auto state = recipientStateDoc.getState(); + + switch (state) { + case TenantMigrationRecipientStateEnum::kUninitialized: + break; + case TenantMigrationRecipientStateEnum::kStarted: + tenant_migration_access_blocker::addTenantMigrationRecipientAccessBlocker( + opCtx->getServiceContext(), + recipientStateDoc.getTenantId(), + recipientStateDoc.getId()); + break; + case TenantMigrationRecipientStateEnum::kConsistent: + if (recipientStateDoc.getRejectReadsBeforeTimestamp()) { + onSetRejectReadsBeforeTimestamp(opCtx, recipientStateDoc); + } + break; + case TenantMigrationRecipientStateEnum::kDone: + break; + default: + MONGO_UNREACHABLE_TASSERT(6112900); + } +} + +void handleShardMergeStateChange(OperationContext* opCtx, + const TenantMigrationRecipientDocument& recipientStateDoc) { + auto state = recipientStateDoc.getState(); + + auto fileImporter = repl::TenantFileImporterService::get(opCtx->getServiceContext()); + + switch (state) { + case TenantMigrationRecipientStateEnum::kUninitialized: + break; + case TenantMigrationRecipientStateEnum::kStarted: + fileImporter->startMigration(recipientStateDoc.getId(), + recipientStateDoc.getDonorConnectionString()); + break; + case TenantMigrationRecipientStateEnum::kLearnedFilenames: + fileImporter->learnedAllFilenames(recipientStateDoc.getId()); + break; + case TenantMigrationRecipientStateEnum::kConsistent: + if (recipientStateDoc.getRejectReadsBeforeTimestamp()) { + onSetRejectReadsBeforeTimestamp(opCtx, recipientStateDoc); + } + break; + case TenantMigrationRecipientStateEnum::kDone: + break; } } } // namespace @@ -153,81 +202,52 @@ void TenantMigrationRecipientOpObserver::onUpdate(OperationContext* opCtx, auto recipientStateDoc = TenantMigrationRecipientDocument::parse( IDLParserContext("recipientStateDoc"), args.updateArgs->updatedDoc); opCtx->recoveryUnit()->onCommit([opCtx, recipientStateDoc](boost::optional<Timestamp>) { - auto mtab = tenant_migration_access_blocker::getTenantMigrationRecipientAccessBlocker( - opCtx->getServiceContext(), recipientStateDoc.getTenantId()); - - if (recipientStateDoc.getExpireAt() && mtab) { - if (mtab->inStateReject()) { - // The TenantMigrationRecipientAccessBlocker entry needs to be removed to - // re-allow reads and future migrations with the same tenantId as this - // migration has already been aborted and forgotten. - TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) - .remove(recipientStateDoc.getTenantId(), - TenantMigrationAccessBlocker::BlockerType::kRecipient); - return; - } - // Once the state doc is marked garbage collectable the TTL deletions should be - // unblocked. - mtab->stopBlockingTTL(); - } - - auto state = recipientStateDoc.getState(); - auto protocol = recipientStateDoc.getProtocol().value_or(kDefaultMigrationProtocol); - if (state == TenantMigrationRecipientStateEnum::kLearnedFilenames) { - tassert(6112900, - "Bad state '{}' for protocol '{}'"_format( - TenantMigrationRecipientState_serializer(state), - MigrationProtocol_serializer(protocol)), - protocol == MigrationProtocolEnum::kShardMerge); - } + if (recipientStateDoc.getExpireAt()) { + repl::TenantFileImporterService::get(opCtx->getServiceContext()) + ->interrupt(recipientStateDoc.getId()); - switch (state) { - case TenantMigrationRecipientStateEnum::kUninitialized: - break; - case TenantMigrationRecipientStateEnum::kStarted: - if (protocol == MigrationProtocolEnum::kMultitenantMigrations) { - tenant_migration_access_blocker::addTenantMigrationRecipientAccessBlocker( - opCtx->getServiceContext(), - recipientStateDoc.getTenantId().toString(), - recipientStateDoc.getId()); - } - break; - case TenantMigrationRecipientStateEnum::kLearnedFilenames: - break; - case TenantMigrationRecipientStateEnum::kConsistent: - if (recipientStateDoc.getRejectReadsBeforeTimestamp()) { - onSetRejectReadsBeforeTimestamp(opCtx, recipientStateDoc); - } - break; - case TenantMigrationRecipientStateEnum::kDone: - break; - } + std::vector<std::string> tenantIdsToRemove; + auto cleanUpBlockerIfGarbage = + [&](std::string tenantId, std::shared_ptr<TenantMigrationAccessBlocker>& mtab) { + if (recipientStateDoc.getId() != mtab->getMigrationId()) { + return; + } + + auto recipientMtab = + checked_pointer_cast<TenantMigrationRecipientAccessBlocker>(mtab); + if (recipientMtab->inStateReject()) { + // The TenantMigrationRecipientAccessBlocker entry needs to be removed + // to re-allow reads and future migrations with the same tenantId as + // this migration has already been aborted and forgotten. + tenantIdsToRemove.push_back(tenantId); + return; + } + // Once the state doc is marked garbage collectable the TTL deletions should + // be unblocked. + recipientMtab->stopBlockingTTL(); + }; - if (protocol != MigrationProtocolEnum::kShardMerge) { - return; - } + TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) + .applyAll(TenantMigrationAccessBlocker::BlockerType::kRecipient, + cleanUpBlockerIfGarbage); - if (recipientStateDoc.getExpireAt() && mtab) { - repl::TenantFileImporterService::get(opCtx->getServiceContext()) - ->interrupt(recipientStateDoc.getId()); + for (const auto& tenantId : tenantIdsToRemove) { + // TODO SERVER-68799: Remove TenantMigrationAccessBlocker removal logic. + TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) + .remove(tenantId, TenantMigrationAccessBlocker::BlockerType::kRecipient); + } } - auto fileImporter = repl::TenantFileImporterService::get(opCtx->getServiceContext()); - - switch (state) { - case TenantMigrationRecipientStateEnum::kUninitialized: - break; - case TenantMigrationRecipientStateEnum::kStarted: - fileImporter->startMigration(recipientStateDoc.getId(), - recipientStateDoc.getDonorConnectionString()); - break; - case TenantMigrationRecipientStateEnum::kLearnedFilenames: - fileImporter->learnedAllFilenames(recipientStateDoc.getId()); - break; - case TenantMigrationRecipientStateEnum::kConsistent: + auto protocol = recipientStateDoc.getProtocol().value_or(kDefaultMigrationProtocol); + switch (protocol) { + case MigrationProtocolEnum::kMultitenantMigrations: + handleMTMStateChange(opCtx, recipientStateDoc); break; - case TenantMigrationRecipientStateEnum::kDone: + case MigrationProtocolEnum::kShardMerge: + handleShardMergeStateChange(opCtx, recipientStateDoc); break; + default: + MONGO_UNREACHABLE; } }); } @@ -252,20 +272,8 @@ void TenantMigrationRecipientOpObserver::aboutToDelete(OperationContext* opCtx, // kDone in order to avoid creating an unnecessary TenantMigrationRecipientAccessBlocker. // In this case, the TenantMigrationRecipientAccessBlocker will not exist for a given // tenant. - auto getTenantId = [&]() -> boost::optional<std::string> { - if (recipientStateDoc.getProtocol() == MigrationProtocolEnum::kMultitenantMigrations) { - auto mtab = - tenant_migration_access_blocker::getTenantMigrationRecipientAccessBlocker( - opCtx->getServiceContext(), recipientStateDoc.getTenantId()); - return mtab ? boost::make_optional(recipientStateDoc.getTenantId().toString()) - : boost::none; - } - - return boost::none; - }; - tenantMigrationInfo(opCtx) = - boost::make_optional(TenantMigrationInfo(recipientStateDoc.getId(), getTenantId())); + boost::make_optional(TenantMigrationInfo(recipientStateDoc.getId())); } } @@ -281,27 +289,17 @@ void TenantMigrationRecipientOpObserver::onDelete(OperationContext* opCtx, return; } - if (tmi->tenantId) { - auto tenantId = tmi->tenantId.get(); - LOGV2_INFO(8423337, - "Removing expired 'multitenant migration' migration", - "tenantId"_attr = tenantId); - opCtx->recoveryUnit()->onCommit([opCtx, tenantId](boost::optional<Timestamp>) { - TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) - .remove(tenantId, TenantMigrationAccessBlocker::BlockerType::kRecipient); - }); - } else { - auto migrationId = tmi->uuid; + auto migrationId = tmi->uuid; + opCtx->recoveryUnit()->onCommit([opCtx, migrationId](boost::optional<Timestamp>) { LOGV2_INFO(6114101, - "Removing expired 'shard merge' migration", + "Removing expired migration access blocker", "migrationId"_attr = migrationId); - opCtx->recoveryUnit()->onCommit([opCtx, migrationId](boost::optional<Timestamp>) { - TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) - .removeRecipientAccessBlockersForMigration(migrationId); - repl::TenantFileImporterService::get(opCtx->getServiceContext()) - ->interrupt(migrationId); - }); - } + repl::TenantFileImporterService::get(opCtx->getServiceContext()) + ->interrupt(migrationId); + TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) + .removeAccessBlockersForMigration( + migrationId, TenantMigrationAccessBlocker::BlockerType::kRecipient); + }); } } @@ -313,10 +311,9 @@ repl::OpTime TenantMigrationRecipientOpObserver::onDropCollection( const CollectionDropType dropType) { if (collectionName == NamespaceString::kTenantMigrationRecipientsNamespace) { opCtx->recoveryUnit()->onCommit([opCtx](boost::optional<Timestamp>) { + repl::TenantFileImporterService::get(opCtx->getServiceContext())->interruptAll(); TenantMigrationAccessBlockerRegistry::get(opCtx->getServiceContext()) .removeAll(TenantMigrationAccessBlocker::BlockerType::kRecipient); - - repl::TenantFileImporterService::get(opCtx->getServiceContext())->interruptAll(); }); } return {}; diff --git a/src/mongo/db/repl/tenant_migration_recipient_service.cpp b/src/mongo/db/repl/tenant_migration_recipient_service.cpp index 427f5410e15..575b4ed4e7e 100644 --- a/src/mongo/db/repl/tenant_migration_recipient_service.cpp +++ b/src/mongo/db/repl/tenant_migration_recipient_service.cpp @@ -325,6 +325,11 @@ void TenantMigrationRecipientService::checkIfConflictsWithOtherInstances( const std::vector<const PrimaryOnlyService::Instance*>& existingInstances) { auto tenantId = initialStateDoc["tenantId"].valueStringData(); + auto recipientStateDocument = TenantMigrationRecipientDocument::parse( + IDLParserContext("recipientStateDoc"), initialStateDoc); + auto protocol = recipientStateDocument.getProtocol().value_or( + MigrationProtocolEnum::kMultitenantMigrations); + for (auto& instance : existingInstances) { auto existingTypedInstance = checked_cast<const TenantMigrationRecipientService::Instance*>(instance); @@ -332,15 +337,24 @@ void TenantMigrationRecipientService::checkIfConflictsWithOtherInstances( auto isDone = existingState.getState() == TenantMigrationRecipientStateEnum::kDone && existingState.getExpireAt(); + if (isDone) { + continue; + } + uassert(ErrorCodes::ConflictingOperationInProgress, "an existing shard merge is in progress", - isDone || - (existingTypedInstance->getProtocol() != MigrationProtocolEnum::kShardMerge && - existingState.getProtocol() != MigrationProtocolEnum::kShardMerge)); + existingTypedInstance->getProtocol() != MigrationProtocolEnum::kShardMerge); + + uassert(ErrorCodes::ConflictingOperationInProgress, + str::stream() << "cannot start " + << MigrationProtocol_serializer(MigrationProtocolEnum::kShardMerge) + << " migration, tenant " << existingTypedInstance->getTenantId() + << " is already migrating", + protocol != MigrationProtocolEnum::kShardMerge); uassert(ErrorCodes::ConflictingOperationInProgress, str::stream() << "tenant " << tenantId << " is already migrating", - isDone || existingTypedInstance->getTenantId() != tenantId); + existingTypedInstance->getTenantId() != tenantId); } } @@ -399,7 +413,9 @@ boost::optional<BSONObj> TenantMigrationRecipientService::Instance::reportForCur stdx::lock_guard lk(_mutex); bob.append("desc", "tenant recipient migration"); _migrationUuid.appendToBuilder(&bob, "instanceID"_sd); - bob.append("tenantId", _stateDoc.getTenantId()); + if (getProtocol() == MigrationProtocolEnum::kMultitenantMigrations) { + bob.append("tenantId", _stateDoc.getTenantId()); + } bob.append("donorConnectionString", _stateDoc.getDonorConnectionString()); bob.append("readPreference", _stateDoc.getReadPreference().toInnerBSON()); bob.append("state", _stateDoc.getState()); @@ -2661,7 +2677,7 @@ SemiFuture<void> TenantMigrationRecipientService::Instance::run( LOGV2(4879607, "Starting tenant migration recipient instance: ", "migrationId"_attr = getMigrationUUID(), - "protocol"_attr = getProtocol(), + "protocol"_attr = MigrationProtocol_serializer(getProtocol()), "tenantId"_attr = getTenantId(), "connectionString"_attr = _donorConnectionString, "readPreference"_attr = _readPreference); diff --git a/src/mongo/db/repl/tenant_migration_state_machine.idl b/src/mongo/db/repl/tenant_migration_state_machine.idl index bdc4b1dd442..1bf94a667ef 100644 --- a/src/mongo/db/repl/tenant_migration_state_machine.idl +++ b/src/mongo/db/repl/tenant_migration_state_machine.idl @@ -169,7 +169,7 @@ structs: startMigrationDonorTimestamp: type: timestamp description: >- - Timestamp after all index builds for tenant are complete or aborted. Recipient + Timestamp after all index builds for tenant are complete or aborted. Recipient must not start cloning/fetching oplog entries from the donor until this timestamp is majority committed. validator: diff --git a/src/mongo/db/repl/tenant_migration_util.h b/src/mongo/db/repl/tenant_migration_util.h index 38d076070af..9fcdb2e1c8a 100644 --- a/src/mongo/db/repl/tenant_migration_util.h +++ b/src/mongo/db/repl/tenant_migration_util.h @@ -51,7 +51,7 @@ constexpr auto kDefaultMigrationProtocol = MigrationProtocolEnum::kMultitenantMi namespace { -const std::set<std::string> kUnsupportedTenantIds{"", "admin", "local", "config"}; +const std::set<std::string> kUnsupportedTenantIds{"admin", "local", "config"}; } // namespace @@ -158,26 +158,26 @@ inline Status validatePrivateKeyPEMPayload(const StringData& payload) { #endif } -inline Status protocolTenantIdCompatibilityCheck(const MigrationProtocolEnum& protocol, - const std::string& tenantId) noexcept { +inline void protocolTenantIdCompatibilityCheck(const MigrationProtocolEnum& protocol, + const std::string& tenantId) { switch (protocol) { case MigrationProtocolEnum::kShardMerge: { - // TODO SERVER-63454: Add a check to ensure tenantId is not provided for 'Merge' - // protocol. + uassert(ErrorCodes::InvalidOptions, + str::stream() << "'tenantId' must be empty for protocol '" + << MigrationProtocol_serializer(protocol) << "'", + tenantId.empty()); break; } case MigrationProtocolEnum::kMultitenantMigrations: { - if (tenantId.empty()) { - return Status(ErrorCodes::InvalidOptions, - str::stream() << "'tenantId' is required for protocol '" - << MigrationProtocol_serializer(protocol) << "'"); - } + uassert(ErrorCodes::InvalidOptions, + str::stream() << "'tenantId' is required for protocol '" + << MigrationProtocol_serializer(protocol) << "'", + !tenantId.empty()); break; } default: MONGO_UNREACHABLE; } - return Status::OK(); } inline void protocolStorageOptionsCompatibilityCheck(OperationContext* opCtx, |