diff options
author | Lingzhi Deng <lingzhi.deng@mongodb.com> | 2019-04-09 09:58:04 -0400 |
---|---|---|
committer | Lingzhi Deng <lingzhi.deng@mongodb.com> | 2019-04-09 09:58:04 -0400 |
commit | 67e08418c8e9722768ead723bafda7e229660291 (patch) | |
tree | 541192d94951c97d14947fd5432f3a38cec4d4f9 | |
parent | 3150154574757e70f1499e97456956b5376630c8 (diff) | |
download | mongo-67e08418c8e9722768ead723bafda7e229660291.tar.gz |
SERVER-40269: commitTransaction should assert that the prepare oplog entry is majority committed
13 files changed, 110 insertions, 67 deletions
diff --git a/jstests/auth/prepared_transaction.js b/jstests/auth/prepared_transaction.js index e93a9e84004..2864dea5fd5 100644 --- a/jstests/auth/prepared_transaction.js +++ b/jstests/auth/prepared_transaction.js @@ -54,7 +54,8 @@ lsid: lsid, txnNumber: NumberLong(0), stmtId: NumberInt(1), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} }), ErrorCodes.Unauthorized); @@ -65,7 +66,8 @@ lsid: lsid, txnNumber: NumberLong(0), stmtId: NumberInt(1), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} })) .prepareTimestamp; const commitTimestamp = Timestamp(prepareTimestamp.getTime(), prepareTimestamp.getInc() + 1); @@ -105,7 +107,8 @@ lsid: lsid, txnNumber: NumberLong(1), stmtId: NumberInt(1), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} }), ErrorCodes.Unauthorized); @@ -115,7 +118,8 @@ lsid: lsid, txnNumber: NumberLong(1), stmtId: NumberInt(1), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} }), ErrorCodes.Unauthorized); @@ -165,7 +169,8 @@ lsid: lsid, txnNumber: NumberLong(2), stmtId: NumberInt(0), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} }), ErrorCodes.Unauthorized); @@ -175,7 +180,8 @@ lsid: lsid, txnNumber: NumberLong(2), stmtId: NumberInt(0), - autocommit: false + autocommit: false, + writeConcern: {w: "majority"} }), ErrorCodes.Unauthorized); diff --git a/jstests/core/txns/commit_prepared_transaction_errors.js b/jstests/core/txns/commit_prepared_transaction_errors.js index 654e9bc8b55..f8f5e27dc6e 100644 --- a/jstests/core/txns/commit_prepared_transaction_errors.js +++ b/jstests/core/txns/commit_prepared_transaction_errors.js @@ -54,8 +54,9 @@ jsTestLog("Test committing an unprepared transaction with a 'commitTimestamp'."); session.startTransaction(); assert.commandWorked(sessionColl.insert(doc)); - assert.commandFailedWithCode(PrepareHelpers.commitTransaction(session, Timestamp(3, 3)), - ErrorCodes.InvalidOptions); + let res = assert.commandFailedWithCode( + PrepareHelpers.commitTransaction(session, Timestamp(3, 3)), ErrorCodes.InvalidOptions); + assert(res.errmsg.includes("cannot provide commitTimestamp to unprepared transaction"), res); jsTestLog("Test committing an unprepared transaction with a null 'commitTimestamp'."); session.startTransaction(); diff --git a/jstests/core/txns/libs/prepare_helpers.js b/jstests/core/txns/libs/prepare_helpers.js index 95efbc8bbea..1bbabf2d224 100644 --- a/jstests/core/txns/libs/prepare_helpers.js +++ b/jstests/core/txns/libs/prepare_helpers.js @@ -12,11 +12,11 @@ const PrepareHelpers = (function() { * * @return {Timestamp} the transaction's prepareTimestamp */ - function prepareTransaction(session) { + function prepareTransaction(session, writeConcernOption = {w: "majority"}) { assert(session); - const res = assert.commandWorked( - session.getDatabase('admin').adminCommand({prepareTransaction: 1})); + const res = assert.commandWorked(session.getDatabase('admin').adminCommand( + {prepareTransaction: 1, writeConcern: writeConcernOption})); assert(res.prepareTimestamp, "prepareTransaction did not return a 'prepareTimestamp': " + tojson(res)); const prepareTimestamp = res.prepareTimestamp; diff --git a/jstests/multiVersion/config_transactions_set_fcv.js b/jstests/multiVersion/config_transactions_set_fcv.js index 4d7fd204462..c5dd323c580 100644 --- a/jstests/multiVersion/config_transactions_set_fcv.js +++ b/jstests/multiVersion/config_transactions_set_fcv.js @@ -43,8 +43,11 @@ } if (prepare) { - const prepareRes = testDB.adminCommand( - {prepareTransaction: 1, txnNumber: NumberLong(txnNumber), lsid, autocommit}); + const prepareRes = testDB.adminCommand({ + prepareTransaction: 1, + txnNumber: NumberLong(txnNumber), lsid, autocommit, + writeConcern: {w: "majority"} + }); if (!prepareRes.ok) { return prepareRes; } diff --git a/jstests/replsets/commit_transaction_recovery.js b/jstests/replsets/commit_transaction_recovery.js index 1f40a13938a..dea4b96de37 100644 --- a/jstests/replsets/commit_transaction_recovery.js +++ b/jstests/replsets/commit_transaction_recovery.js @@ -28,18 +28,16 @@ let sessionDB = session.getDatabase(dbName); const sessionColl = sessionDB.getCollection(collName); - jsTestLog("Disable snapshotting on all nodes"); + session.startTransaction(); + assert.commandWorked(sessionColl.insert({_id: 1})); + let prepareTimestamp = PrepareHelpers.prepareTransaction(session); + jsTestLog("Disable snapshotting on all nodes"); // Disable snapshotting so that future operations do not enter the majority snapshot. assert.commandWorked( primary.adminCommand({configureFailPoint: "disableSnapshotting", mode: "alwaysOn"})); - session.startTransaction(); - assert.commandWorked(sessionColl.insert({_id: 1})); - let prepareTimestamp = PrepareHelpers.prepareTransaction(session); - jsTestLog("Committing the transaction"); - // Since the commitTimestamp is after the last snapshot, this oplog entry will be replayed // during replication recovery during restart. assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); @@ -69,4 +67,4 @@ assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 1}); replTest.stopSet(); -}());
\ No newline at end of file +}()); diff --git a/jstests/replsets/prepare_transaction_index_build.js b/jstests/replsets/prepare_transaction_index_build.js index 7f495ddceed..543d9b919e9 100644 --- a/jstests/replsets/prepare_transaction_index_build.js +++ b/jstests/replsets/prepare_transaction_index_build.js @@ -47,7 +47,7 @@ session.startTransaction(); assert.commandWorked(sessionColl.insert({x: 1000})); - const prepareTimestamp = PrepareHelpers.prepareTransaction(session); + const prepareTimestamp = PrepareHelpers.prepareTransaction(session, {w: 1}); jsTestLog("Unblocking index build."); diff --git a/jstests/replsets/recover_committed_aborted_prepared_transactions.js b/jstests/replsets/recover_committed_aborted_prepared_transactions.js index 561e9e4eb2c..9b0d83fc0e7 100644 --- a/jstests/replsets/recover_committed_aborted_prepared_transactions.js +++ b/jstests/replsets/recover_committed_aborted_prepared_transactions.js @@ -1,6 +1,8 @@ /** - * Test that rollback can successfully recover committed and aborted prepared transactions - * that were prepared and committed/aborted between the stable timestamp and the common point. + * 1. Test that rollback can successfully recover committed prepared transactions that were prepared + * before the stable timestamp and committed between the stable timestamp and the common point. + * 2. Test that rollback can successfully recover aborted prepared transactions that were prepared + * and aborted between the stable timestamp and the common point. * * @tags: [uses_transactions, uses_prepare_transaction] */ @@ -37,6 +39,12 @@ assert.commandWorked(sessionColl1.insert({id: 1})); rollbackTest.awaitLastOpCommitted(); + + // Prepare a transaction on the first session which will be committed eventually. + session1.startTransaction(); + assert.commandWorked(sessionColl1.insert({id: 2})); + const prepareTimestamp = PrepareHelpers.prepareTransaction(session1); + // Prevent the stable timestamp from moving beyond the following prepared transactions so // that when we replay the oplog from the stable timestamp, we correctly recover them. assert.commandWorked( @@ -45,15 +53,10 @@ // The following transactions will be prepared before the common point, so they must be in // prepare after rollback recovery. - // Prepare a transaction on the first session. - session1.startTransaction(); - assert.commandWorked(sessionColl1.insert({id: 2})); - const prepareTimestamp = PrepareHelpers.prepareTransaction(session1); - - // Prepare another transaction on the second session. + // Prepare another transaction on the second session which will be aborted. session2.startTransaction(); assert.commandWorked(sessionColl2.insert({id: 3})); - const prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2); + const prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2, {w: 1}); // Commit the first transaction. assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp)); diff --git a/jstests/replsets/recover_multiple_prepared_transactions_startup.js b/jstests/replsets/recover_multiple_prepared_transactions_startup.js index 32fe036da15..ae90d4708f0 100644 --- a/jstests/replsets/recover_multiple_prepared_transactions_startup.js +++ b/jstests/replsets/recover_multiple_prepared_transactions_startup.js @@ -42,11 +42,11 @@ session.startTransaction(); assert.commandWorked(sessionColl.update({_id: 1}, {_id: 1, a: 1})); - let prepareTimestamp = PrepareHelpers.prepareTransaction(session); + let prepareTimestamp = PrepareHelpers.prepareTransaction(session, {w: 1}); session2.startTransaction(); assert.commandWorked(sessionColl2.update({_id: 2}, {_id: 2, a: 1})); - let prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2); + let prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2, {w: 1}); const lsid = session.getSessionId(); const txnNumber = session.getTxnNumber_forTesting(); @@ -156,4 +156,4 @@ assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 3}); replTest.stopSet(); -}());
\ No newline at end of file +}()); diff --git a/jstests/replsets/recover_prepared_transaction_state.js b/jstests/replsets/recover_prepared_transaction_state.js index e78e68169ec..6b7888ea3a2 100644 --- a/jstests/replsets/recover_prepared_transaction_state.js +++ b/jstests/replsets/recover_prepared_transaction_state.js @@ -1,14 +1,16 @@ /** - * Test that rollback can successfully recover a prepared transaction that was in - * prepare between the stable timestamp and the common point. + * 1. Test that rollback can successfully recover an aborted prepared transaction that was + * rolled-back but was in prepare between the stable timestamp and the common point. + * 2. Test that rollback can successfully recover a committed prepared transaction that was + * rolled-back but was in prepare before the stable timestamp. * * This test holds back the stable timestamp and starts two prepared transactions * before transitioning to rollback operations, where the branches of history on * the rollback node and sync source will diverge. This ensures that we prepare * the transactions in between the stable timestamp and the common point. * - * After a rollback, we should correctly reconstruct the two prepared transactions - * and be able to commit/abort them. + * After a rollback of commit/abort, we should correctly reconstruct the two prepared transactions + * and be able to commit/abort them again. * * @tags: [uses_transactions, uses_prepare_transaction] */ @@ -51,6 +53,13 @@ assert.commandWorked(sessionColl1.insert({_id: 2})); rollbackTest.awaitLastOpCommitted(); + + // Prepare a transaction on the first session whose commit will be rolled-back. + session1.startTransaction(); + assert.commandWorked(sessionColl1.insert({_id: 3})); + assert.commandWorked(sessionColl1.update({_id: 1}, {$set: {a: 1}})); + const prepareTimestamp = PrepareHelpers.prepareTransaction(session1); + // Prevent the stable timestamp from moving beyond the following prepared transactions so // that when we replay the oplog from the stable timestamp, we correctly recover them. assert.commandWorked( @@ -59,18 +68,11 @@ // The following transactions will be prepared before the common point, so they must be in // prepare after rollback recovery. - // Prepare a transaction on the first session. - session1.startTransaction(); - assert.commandWorked(sessionColl1.insert({_id: 3})); - assert.commandWorked(sessionColl1.update({_id: 1}, {$set: {a: 1}})); - const prepareTimestamp = PrepareHelpers.prepareTransaction(session1); - - // Prepare another transaction on the second session. + // Prepare another transaction on the second session whose abort will be rolled-back. session2.startTransaction(); - assert.commandWorked(sessionColl2.insert({_id: 4})); assert.commandWorked(sessionColl2.update({_id: 2}, {$set: {b: 2}})); - const prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2); + const prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2, {w: 1}); // Check that we have two transactions in the transactions table. assert.eq(primary.getDB('config')['transactions'].find().itcount(), 2); diff --git a/jstests/replsets/rollback_aborted_prepared_transaction.js b/jstests/replsets/rollback_aborted_prepared_transaction.js index 3093582aede..8c47034901a 100644 --- a/jstests/replsets/rollback_aborted_prepared_transaction.js +++ b/jstests/replsets/rollback_aborted_prepared_transaction.js @@ -1,5 +1,9 @@ /** * Test that rollback can successfully roll back an aborted prepared transaction. + * Also test that it is impossible to commit a prepared transaction whose prepare oplog entry has + * not yet been majority committed. This case should never happen when prepareTransaction is driven + * by the transaction coordinator because transaction coordinator should always call + * prepareTransaction with {w : "majority"}. * * @tags: [uses_transactions, uses_prepare_transaction] */ @@ -23,33 +27,49 @@ assert.commandWorked(testDB.runCommand({create: collName})); assert.commandWorked(testColl.insert({_id: 0})); - // Start a session on the primary. + // Start two sessions on the primary. let session = primary.startSession(); const sessionID = session.getSessionId(); let sessionDB = session.getDatabase(dbName); let sessionColl = sessionDB.getCollection(collName); + let session2 = primary.startSession(); + let sessionColl2 = session2.getDatabase(dbName).getCollection(collName); + // The following transaction will be rolled back. rollbackTest.transitionToRollbackOperations(); // Prepare the transaction on the session. session.startTransaction(); assert.commandWorked(sessionColl.insert({_id: 1})); - PrepareHelpers.prepareTransaction(session); + PrepareHelpers.prepareTransaction(session, {w: 1}); assert.eq(testColl.find().itcount(), 1); // This characterizes the current fastcount behavior, which is that active prepared transactions // contribute to the fastcount. assert.eq(testColl.count(), 2); - // Abort the transaction. + // Abort the transaction explicitly. session.abortTransaction_forTesting(); assert.eq(testColl.find().itcount(), 1); assert.eq(testColl.count(), 1); - // Check that we have one transaction in the transactions table. - assert.eq(primary.getDB('config')['transactions'].find().itcount(), 1); + // Test that it is impossible to commit a prepared transaction whose prepare oplog entry has not + // yet majority committed. This also aborts the transaction. + session2.startTransaction(); + assert.commandWorked(sessionColl2.insert({_id: 2})); + let prepareTimestamp = PrepareHelpers.prepareTransaction(session2, {w: 1}); + let res = assert.commandFailedWithCode( + PrepareHelpers.commitTransaction(session2, prepareTimestamp), ErrorCodes.InvalidOptions); + assert(res.errmsg.includes( + "cannot be run before its prepare oplog entry has been majority committed"), + res); + assert.eq(testColl.find().itcount(), 1); + assert.eq(testColl.count(), 1); + + // Check that we have two transactions in the transactions table. + assert.eq(primary.getDB('config')['transactions'].find().itcount(), 2); rollbackTest.transitionToSyncSourceOperationsBeforeRollback(); rollbackTest.transitionToSyncSourceOperationsDuringRollback(); @@ -78,7 +98,7 @@ session.startTransaction(); assert.commandWorked(sessionColl.insert({_id: 1})); - const prepareTimestamp = PrepareHelpers.prepareTransaction(session); + prepareTimestamp = PrepareHelpers.prepareTransaction(session); PrepareHelpers.commitTransaction(session, prepareTimestamp); assert.eq(testColl.find().itcount(), 2); diff --git a/jstests/replsets/rollback_via_refetch_commit_transaction.js b/jstests/replsets/rollback_via_refetch_commit_transaction.js index bb4ca534553..380bcdb4fd2 100644 --- a/jstests/replsets/rollback_via_refetch_commit_transaction.js +++ b/jstests/replsets/rollback_via_refetch_commit_transaction.js @@ -31,26 +31,25 @@ TestData.skipCheckDBHashes = true; config.settings = {chainingAllowed: false}; rst.initiate(config); - const rollbackTest = new RollbackTest(collName, rst); + const primaryNode = rst.getPrimary(); // Create collection that exists on the sync source and rollback node. - assert.commandWorked(rollbackTest.getPrimary().getDB(dbName).runCommand( - {create: collName, writeConcern: {w: 2}})); - - // Stop replication from the current primary ("rollbackNode"). - const rollbackNode = rollbackTest.transitionToRollbackOperations(); + assert.commandWorked( + primaryNode.getDB(dbName).runCommand({create: collName, writeConcern: {w: 2}})); // Issue a 'prepareTransaction' command just to the current primary. - const session = rollbackNode.getDB(dbName).getMongo().startSession({causalConsistency: false}); + const session = primaryNode.getDB(dbName).getMongo().startSession({causalConsistency: false}); const sessionDB = session.getDatabase(dbName); const sessionColl = sessionDB.getCollection(collName); session.startTransaction(); assert.commandWorked(sessionColl.insert({"prepare": "entry"})); - const result = assert.commandWorked( - session.getDatabase('admin').adminCommand({prepareTransaction: 1, writeConcern: {w: 1}})); - assert(result.prepareTimestamp, - "prepareTransaction did not return a 'prepareTimestamp': " + tojson(result)); - PrepareHelpers.commitTransaction(session, result.prepareTimestamp); + const prepareTimestamp = PrepareHelpers.prepareTransaction(session); + + const rollbackTest = new RollbackTest(collName, rst); + // Stop replication from the current primary ("rollbackNode"). + const rollbackNode = rollbackTest.transitionToRollbackOperations(); + + PrepareHelpers.commitTransaction(session, prepareTimestamp); // Step down current primary and elect a node that lacks the commit. rollbackTest.transitionToSyncSourceOperationsBeforeRollback(); @@ -76,5 +75,6 @@ TestData.skipCheckDBHashes = true; return rawMongoProgramOutput().match(msg); }, "Node did not fail to roll back entry."); - rst.stopSet(); + // Transaction is still in prepared state and validation will be blocked, so skip it. + rst.stopSet(undefined, undefined, {skipValidation: true}); }()); diff --git a/jstests/sharding/aggregation_currentop.js b/jstests/sharding/aggregation_currentop.js index c500da1644e..5e7ed32f09a 100644 --- a/jstests/sharding/aggregation_currentop.js +++ b/jstests/sharding/aggregation_currentop.js @@ -759,8 +759,12 @@ TestData.skipAwaitingReplicationOnShardsBeforeCheckingUUIDs = true; 0); // Prepare the transaction and ensure the prepareTimestamp is valid. - const prepareRes = assert.commandWorked(sessionDB.adminCommand( - {prepareTransaction: 1, txnNumber: NumberLong(0), autocommit: false})); + const prepareRes = assert.commandWorked(sessionDB.adminCommand({ + prepareTransaction: 1, + txnNumber: NumberLong(0), + autocommit: false, + writeConcern: {w: "majority"} + })); assert(prepareRes.prepareTimestamp, "prepareTransaction did not return a 'prepareTimestamp': " + tojson(prepareRes)); assert(prepareRes.prepareTimestamp instanceof Timestamp, diff --git a/src/mongo/db/commands/txn_cmds.cpp b/src/mongo/db/commands/txn_cmds.cpp index ca3cf1edf62..477eddcf405 100644 --- a/src/mongo/db/commands/txn_cmds.cpp +++ b/src/mongo/db/commands/txn_cmds.cpp @@ -116,6 +116,12 @@ public: auto optionalCommitTimestamp = cmd.getCommitTimestamp(); if (optionalCommitTimestamp) { + const auto replCoord = repl::ReplicationCoordinator::get(opCtx); + uassert(ErrorCodes::InvalidOptions, + "commitTransaction for a prepared transaction cannot be run before its prepare " + "oplog entry has been majority committed", + replCoord->getLastCommittedOpTime().getTimestamp() >= + txnParticipant.getPrepareOpTime().getTimestamp()); // commitPreparedTransaction will throw if the transaction is not prepared. txnParticipant.commitPreparedTransaction(opCtx, optionalCommitTimestamp.get(), {}); } else { |