From 7eb93258d074ee3bac55e0a79902972f1cb4c89d Mon Sep 17 00:00:00 2001 From: Cheahuychou Mao Date: Thu, 3 Feb 2022 23:50:46 +0000 Subject: SERVER-63071 Prepared internal transactions for retryable findAndModify can cause stepup to hang --- ...d_abort_prepared_transactions_after_failover.js | 184 +++++++++++++++++++++ src/mongo/db/index_build_entry_helpers.cpp | 4 +- 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 jstests/sharding/internal_transactions_for_retryable_findAndModify_commit_and_abort_prepared_transactions_after_failover.js diff --git a/jstests/sharding/internal_transactions_for_retryable_findAndModify_commit_and_abort_prepared_transactions_after_failover.js b/jstests/sharding/internal_transactions_for_retryable_findAndModify_commit_and_abort_prepared_transactions_after_failover.js new file mode 100644 index 00000000000..e29eb472531 --- /dev/null +++ b/jstests/sharding/internal_transactions_for_retryable_findAndModify_commit_and_abort_prepared_transactions_after_failover.js @@ -0,0 +1,184 @@ +/* + * Test that prepared retryable internal transactions with a findAndModify statement can commit and + * abort after failover. + * + * @tags: [requires_fcv_53, featureFlagInternalTransactions] + */ +(function() { +'use strict'; + +// TODO (SERVER-63258): Resolve inconsistency around the write to the image collection +// for prepared internal transactions. +// For the test case where we abort a prepared internal transaction for retryable findAndModify with +// a pre/post image, the image collection on the primary is expected to be inconsistent with the +// image collection on secondaries. The reason is that for prepared transactions, the pre/post image +// is written to the image collection at prepare time, and on the primary the write is done in a +// side storage engine transaction, whereas on secondaries the write is done in the prepared +// transaction's storage engine transaction. Therefore, when the prepared transaction is aborted, +// the write to image collection only gets rolled back on secondaries. +TestData.skipCheckDBHashes = true; + +load("jstests/replsets/rslib.js"); +load("jstests/sharding/libs/sharded_transactions_helpers.js"); + +function runTest(st, stepDownShard0PrimaryFunc, testOpts = { + runFindAndModifyWithPreOrPostImage, + abortTxnAfterFailover, + enableFindAndModifyImageCollection +}) { + jsTest.log("Testing with options " + tojson(testOpts)); + + const sessionUUID = UUID(); + const parentLsid = {id: sessionUUID}; + const parentTxnNumber = NumberLong(35); + const childLsid = {id: parentLsid.id, txnNumber: parentTxnNumber, txnUUID: UUID()}; + const childTxnNumber = NumberLong(0); + const stmtId = NumberInt(1); + + const kDbName = "testDb"; + const kCollName = "testColl-" + sessionUUID; + let testDB = st.rs0.getPrimary().getDB(kDbName); + let testColl = testDB.getCollection(kCollName); + + assert.commandWorked(testDB.adminCommand({ + setParameter: 1, + storeFindAndModifyImagesInSideCollection: testOpts.enableFindAndModifyImageCollection + })); + + assert.commandWorked(testDB.createCollection(kCollName)); + if (testOpts.runFindAndModifyWithPreOrPostImage) { + assert.commandWorked(testColl.insert({_id: 0, x: 0})); + } + + const findAndModifyCmdObj = { + findAndModify: kCollName, + query: {_id: 0, x: 0}, + update: {$inc: {x: 1}}, + upsert: true, + lsid: childLsid, + txnNumber: childTxnNumber, + startTransaction: true, + autocommit: false, + stmtId: stmtId, + }; + const prepareCmdObj = makePrepareTransactionCmdObj(childLsid, childTxnNumber); + const commitCmdObj = makeCommitTransactionCmdObj(childLsid, childTxnNumber); + const abortCmdObj = makeAbortTransactionCmdObj(childLsid, childTxnNumber); + + const initialRes = assert.commandWorked(testDB.runCommand(findAndModifyCmdObj)); + const prepareTxnRes = assert.commandWorked(testDB.adminCommand(prepareCmdObj)); + commitCmdObj.commitTimestamp = prepareTxnRes.prepareTimestamp; + + stepDownShard0PrimaryFunc(); + + testDB = st.rs0.getPrimary().getDB(kDbName); + testColl = testDB.getCollection(kCollName); + + if (testOpts.abortTxnAfterFailover) { + assert.commandWorked(testDB.adminCommand(abortCmdObj)); + assert.eq(testColl.find({_id: 0, x: 1}).itcount(), 0); + } else { + assert.commandWorked(testDB.adminCommand(commitCmdObj)); + assert.eq(testColl.find({_id: 0, x: 1}).itcount(), 1); + + // Test that the findAndModify is retryable after failover. + const retryRes = assert.commandWorked(testDB.runCommand(findAndModifyCmdObj)); + assert.commandWorked(testDB.adminCommand(commitCmdObj)); + assert.eq(initialRes.lastErrorObject, retryRes.lastErrorObject, retryRes); + assert.eq(initialRes.value, retryRes.value); + assert.eq(testColl.find({_id: 0, x: 1}).itcount(), 1); + } +} + +{ + jsTest.log("Test when the old primary steps up"); + const st = new ShardingTest({shards: 1, rs: {nodes: 1}}); + const stepDownShard0PrimaryFunc = () => { + assert.commandWorked(st.rs0.getPrimary().adminCommand({replSetStepDown: 1, force: true})); + }; + + // Test findAnModify without pre/post image. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: false, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: true + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: false, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: true + }); + + // Test findAnModify without pre/post image when the image collection is enabled. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: true + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: true + }); + // Test findAnModify without pre/post image when the image collection is disabled. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: false + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: false + }); + + st.stop(); +} + +{ + jsTest.log("Test when an old secondary steps up"); + const st = new ShardingTest({shards: 1, rs: {nodes: 2}}); + const stepDownShard0PrimaryFunc = () => { + assert.commandWorked(st.rs0.getSecondary().adminCommand({replSetFreeze: 0})); + assert.commandWorked(st.rs0.getPrimary().adminCommand( + {replSetStepDown: ReplSetTest.kForeverSecs, force: true})); + }; + + // Test findAnModify without pre/post image. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: false, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: true + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: false, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: true + }); + + // Test findAnModify without pre/post image when the image collection is enabled. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: true + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: true + }); + // Test findAnModify without pre/post image when the image collection is disabled. + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: false, + enableFindAndModifyImageCollection: false + }); + runTest(st, stepDownShard0PrimaryFunc, { + runFindAndModifyWithPreOrPostImage: true, + abortTxnAfterFailover: true, + enableFindAndModifyImageCollection: false + }); + + st.stop(); +} +})(); diff --git a/src/mongo/db/index_build_entry_helpers.cpp b/src/mongo/db/index_build_entry_helpers.cpp index a8206954697..abae656e504 100644 --- a/src/mongo/db/index_build_entry_helpers.cpp +++ b/src/mongo/db/index_build_entry_helpers.cpp @@ -170,7 +170,7 @@ void ensureIndexBuildEntriesNamespaceExists(OperationContext* opCtx) { "createIndexBuildCollection", NamespaceString::kIndexBuildEntryNamespace.ns(), [&]() -> void { - AutoGetDb autoDb(opCtx, NamespaceString::kIndexBuildEntryNamespace.db(), MODE_X); + AutoGetDb autoDb(opCtx, NamespaceString::kIndexBuildEntryNamespace.db(), MODE_IX); auto db = autoDb.ensureDbExists(opCtx); // Ensure the database exists. @@ -180,6 +180,8 @@ void ensureIndexBuildEntriesNamespaceExists(OperationContext* opCtx) { if (!CollectionCatalog::get(opCtx)->lookupCollectionByNamespace( opCtx, NamespaceString::kIndexBuildEntryNamespace)) { WriteUnitOfWork wuow(opCtx); + AutoGetCollection autoColl( + opCtx, NamespaceString::kIndexBuildEntryNamespace, LockMode::MODE_IX); CollectionOptions defaultCollectionOptions; CollectionPtr collection = db->createCollection( opCtx, NamespaceString::kIndexBuildEntryNamespace, defaultCollectionOptions); -- cgit v1.2.1