diff options
| author | Pavi Vetriselvan <pvselvan@umich.edu> | 2019-04-01 13:42:11 -0400 |
|---|---|---|
| committer | Pavi Vetriselvan <pvselvan@umich.edu> | 2019-04-01 14:05:19 -0400 |
| commit | e433a5aee915568cf73b05e89597903855ed1952 (patch) | |
| tree | 6c05ba3dab362405e8d07cc5bf65ea86003ddbbf | |
| parent | e96547906836a12d76b7a1591a0028b1fff8cb79 (diff) | |
| download | mongo-e433a5aee915568cf73b05e89597903855ed1952.tar.gz | |
Revert "SERVER-35811 disallow committing at the prepareTimestamp and pin stable TS before oldest uncommitted TS"
This reverts commit b58420e57a2149c350ebf82815fe708850402296 and 33ac1afd4079e04d12554f9b79d1ab07426caf59.
41 files changed, 163 insertions, 541 deletions
diff --git a/jstests/concurrency/fsm_workload_helpers/auto_retry_transaction.js b/jstests/concurrency/fsm_workload_helpers/auto_retry_transaction.js index b05323d9f39..02d6cfd5fc7 100644 --- a/jstests/concurrency/fsm_workload_helpers/auto_retry_transaction.js +++ b/jstests/concurrency/fsm_workload_helpers/auto_retry_transaction.js @@ -58,7 +58,7 @@ var {withTxnAndAutoRetry} = (function() { const rand = Random.rand(); if (rand < prepareProbability) { const prepareTimestamp = PrepareHelpers.prepareTransaction(session); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); } else { // commitTransaction() calls assert.commandWorked(), which may fail with a // WriteConflict error response, which is ignored. diff --git a/jstests/core/txns/commit_prepared_transaction.js b/jstests/core/txns/commit_prepared_transaction.js index 79de318d901..c14c9c607bf 100644 --- a/jstests/core/txns/commit_prepared_transaction.js +++ b/jstests/core/txns/commit_prepared_transaction.js @@ -33,7 +33,7 @@ assert.eq(doc1, sessionColl.findOne(doc1)); let prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); // After commit the insert persists. assert.eq(doc1, testColl.findOne(doc1)); @@ -52,7 +52,7 @@ assert.eq(doc2, sessionColl.findOne(doc2)); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); // After commit the update persists. assert.eq(doc2, testColl.findOne({_id: 1})); @@ -69,7 +69,7 @@ assert.eq(null, sessionColl.findOne(doc2)); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); // After commit the delete persists. assert.eq(null, testColl.findOne(doc2)); diff --git a/jstests/core/txns/disallow_operations_on_prepared_transaction.js b/jstests/core/txns/disallow_operations_on_prepared_transaction.js index 6c8bc18b8f8..dcb924e89e8 100644 --- a/jstests/core/txns/disallow_operations_on_prepared_transaction.js +++ b/jstests/core/txns/disallow_operations_on_prepared_transaction.js @@ -34,7 +34,7 @@ session.startTransaction(); assert.commandWorked(sessionColl.insert({_id: 2})); let prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); jsTestLog("Test that you can call abortTransaction on a prepared transaction."); session.startTransaction(); diff --git a/jstests/core/txns/libs/prepare_helpers.js b/jstests/core/txns/libs/prepare_helpers.js index d157fc351ce..4d87fd9ee07 100644 --- a/jstests/core/txns/libs/prepare_helpers.js +++ b/jstests/core/txns/libs/prepare_helpers.js @@ -48,25 +48,6 @@ const PrepareHelpers = (function() { } /** - * Commits the active transaction on the session at a commitTimestamp that is greater than - * the transaction's prepareTimestamp. - * - * This is a temporary function that should be used to commit prepared transactions - * until we allow the stable timestamp to move past the oldest active prepare timestamp. - * - * @return {object} the response to the 'commitTransaction' command. - */ - function commitTransactionAfterPrepareTS(session, prepareTimestamp) { - assert(session); - - // Add 1 to the increment so that the commitTimestamp is "after" the prepareTimestamp. - const commitTimestamp = - Timestamp(prepareTimestamp.getTime(), prepareTimestamp.getInc() + 1); - - return this.commitTransaction(session, commitTimestamp); - } - - /** * Creates a session object on the given connection with the provided 'lsid'. * * @return {session} the session created. @@ -84,7 +65,6 @@ const PrepareHelpers = (function() { return { prepareTransaction: prepareTransaction, commitTransaction: commitTransaction, - commitTransactionAfterPrepareTS: commitTransactionAfterPrepareTS, createSessionWithGivenId: createSessionWithGivenId, }; })(); diff --git a/jstests/core/txns/prepare_conflict_aggregation_behavior.js b/jstests/core/txns/prepare_conflict_aggregation_behavior.js index cd8e114ed0e..0df15e63065 100644 --- a/jstests/core/txns/prepare_conflict_aggregation_behavior.js +++ b/jstests/core/txns/prepare_conflict_aggregation_behavior.js @@ -61,7 +61,7 @@ // Make sure that we can't see the update from the aggregation or the prepared transaction. assert.eq([{_id: 0}, {_id: 1, b: 1}], outColl.find().toArray()); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); // Make sure that the $out pipeline works once the transaction is committed. testColl.aggregate(pipeline); diff --git a/jstests/core/txns/prepare_requires_fcv42.js b/jstests/core/txns/prepare_requires_fcv42.js index bca4dac6812..e7147aa13c6 100644 --- a/jstests/core/txns/prepare_requires_fcv42.js +++ b/jstests/core/txns/prepare_requires_fcv42.js @@ -26,8 +26,7 @@ session.startTransaction(); assert.commandWorked(sessionDB[collName].insert({_id: "a"})); let prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); jsTestLog("Downgrade the featureCompatibilityVersion."); assert.commandWorked(testDB.adminCommand({setFeatureCompatibilityVersion: lastStableFCV})); @@ -51,7 +50,7 @@ session.startTransaction(); assert.commandWorked(sessionDB[collName].insert({_id: "c"})); let prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); session.endSession(); }()); diff --git a/jstests/core/txns/prepared_transactions_do_not_block_non_conflicting_ddl.js b/jstests/core/txns/prepared_transactions_do_not_block_non_conflicting_ddl.js index a5bf806987d..da915154ee8 100644 --- a/jstests/core/txns/prepared_transactions_do_not_block_non_conflicting_ddl.js +++ b/jstests/core/txns/prepared_transactions_do_not_block_non_conflicting_ddl.js @@ -28,8 +28,7 @@ assert.commandWorked(sessionColl.update({_id: 1}, {$inc: {x: 1}})); const prepareTimestamp = PrepareHelpers.prepareTransaction(session); assert.commandWorked(testDB.getSiblingDB(cmdDBName).runCommand(ddlCmd)); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); } jsTest.log("Test 'create'."); diff --git a/jstests/core/txns/timestamped_reads_wait_for_prepare_oplog_visibility.js b/jstests/core/txns/timestamped_reads_wait_for_prepare_oplog_visibility.js index 8423865df55..1f052336ad6 100644 --- a/jstests/core/txns/timestamped_reads_wait_for_prepare_oplog_visibility.js +++ b/jstests/core/txns/timestamped_reads_wait_for_prepare_oplog_visibility.js @@ -83,7 +83,7 @@ db.setLogLevel(0); joinReadThread({checkExitSuccess: true}); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); } const snapshotRead = function(_collName) { diff --git a/jstests/noPassthrough/change_stream_transaction.js b/jstests/noPassthrough/change_stream_transaction.js index b8d14d24a0a..8e41ccc3f6c 100644 --- a/jstests/noPassthrough/change_stream_transaction.js +++ b/jstests/noPassthrough/change_stream_transaction.js @@ -134,8 +134,7 @@ // // Commit first transaction and confirm expected changes. // - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestampTxn1)); + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestampTxn1)); assertWriteVisibleWithCapture( changeStreamCursor, "insert", {_id: "no-txn-doc-3"}, changeList); assertWriteVisibleWithCapture( @@ -192,8 +191,7 @@ prepareTimestampTxn3 = PrepareHelpers.prepareTransaction(session3); assertNoChanges(changeStreamCursor); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session3, prepareTimestampTxn3)); + assert.commandWorked(PrepareHelpers.commitTransaction(session3, prepareTimestampTxn3)); assertNoChanges(changeStreamCursor); assert.commandWorked(db.dropDatabase()); diff --git a/jstests/noPassthrough/currentop_active_transaction.js b/jstests/noPassthrough/currentop_active_transaction.js index 019df41aeba..41c497ecb1b 100644 --- a/jstests/noPassthrough/currentop_active_transaction.js +++ b/jstests/noPassthrough/currentop_active_transaction.js @@ -19,7 +19,7 @@ // Load the prepare helpers to be called in the parallel shell. load('jstests/core/txns/libs/prepare_helpers.js'); const prepareTimestamp = PrepareHelpers.prepareTransaction(session); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); } else { session.commitTransaction(); } diff --git a/jstests/noPassthrough/currentop_transaction_metrics.js b/jstests/noPassthrough/currentop_transaction_metrics.js index 1a023e10cf5..d676167c2c2 100644 --- a/jstests/noPassthrough/currentop_transaction_metrics.js +++ b/jstests/noPassthrough/currentop_transaction_metrics.js @@ -65,7 +65,7 @@ prepareTransactionDocument.timeInactiveMicros); assert.gte(prepareTransactionDocument.timePreparedMicros, 0); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); session.endSession(); rst.stopSet(); diff --git a/jstests/noPassthrough/server_transaction_metrics_for_prepared_transactions.js b/jstests/noPassthrough/server_transaction_metrics_for_prepared_transactions.js index b651e350a30..7a30e520fbe 100644 --- a/jstests/noPassthrough/server_transaction_metrics_for_prepared_transactions.js +++ b/jstests/noPassthrough/server_transaction_metrics_for_prepared_transactions.js @@ -95,7 +95,7 @@ // Verify the total prepared and committed transaction counters are updated after a commit // and that the current prepared counter is decremented. - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestampForCommit); + PrepareHelpers.commitTransaction(session, prepareTimestampForCommit); newStatus = assert.commandWorked(testDB.adminCommand({serverStatus: 1})); verifyServerStatusFields(newStatus); verifyServerStatusChange( diff --git a/jstests/noPassthrough/skip_write_conflict_retries_failpoint.js b/jstests/noPassthrough/skip_write_conflict_retries_failpoint.js index c6e1ef5a5b9..538ba853938 100644 --- a/jstests/noPassthrough/skip_write_conflict_retries_failpoint.js +++ b/jstests/noPassthrough/skip_write_conflict_retries_failpoint.js @@ -56,7 +56,7 @@ assert.commandFailedWithCode(testColl.update({_id: "from transaction"}, {$set: {a: 1}}), ErrorCodes.WriteConflict); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); assert.eq(testColl.findOne({a: 1}), {_id: "from prepared transaction", a: 1}); assert.commandWorked( diff --git a/jstests/replsets/commit_prepared_transaction_before_stable_timestamp.js b/jstests/replsets/commit_prepared_transaction_before_stable_timestamp.js new file mode 100644 index 00000000000..71ca1189b45 --- /dev/null +++ b/jstests/replsets/commit_prepared_transaction_before_stable_timestamp.js @@ -0,0 +1,59 @@ +/** + * Test that we can successfully commit a prepared transaction before the stable timestamp. + * + * @tags: [uses_transactions, uses_prepare_transaction] + */ + +(function() { + "use strict"; + load("jstests/aggregation/extras/utils.js"); + load("jstests/core/txns/libs/prepare_helpers.js"); + + const replTest = new ReplSetTest({nodes: 1}); + replTest.startSet(); + replTest.initiate(); + + const primary = replTest.getPrimary(); + + const dbName = "test"; + const collName = "commit_prepared_transaction_before_stable_timestamp"; + const testDB = primary.getDB(dbName); + const testColl = testDB.getCollection(collName); + + assert.commandWorked(testDB.runCommand({create: collName})); + + // Make sure there is no lag between the oldest timestamp and the stable timestamp so we can + // test that committing a prepared transaction behind the oldest timestamp succeeds. + assert.commandWorked(primary.adminCommand({ + "configureFailPoint": 'WTSetOldestTSToStableTS', + "mode": 'alwaysOn', + })); + + const session = primary.startSession({causalConsistency: false}); + const sessionDB = session.getDatabase(dbName); + const sessionColl = sessionDB.getCollection(collName); + + session.startTransaction(); + assert.commandWorked(sessionColl.insert({_id: 1})); + const prepareTimestamp = PrepareHelpers.prepareTransaction(session); + + jsTestLog("Do a majority write to advance the stable timestamp past the prepareTimestamp"); + // Doing a majority write after preparing the transaction ensures that the stable timestamp is + // past the prepare timestamp because this write must be in the committed snapshot. + assert.commandWorked( + testColl.runCommand("insert", {documents: [{_id: 2}]}, {writeConcern: {w: "majority"}})); + + jsTestLog("Committing the transaction before the stable timestamp"); + + // Since we have advanced the stableTimestamp to be after the prepareTimestamp, when we commit + // at the prepareTimestamp, we are certain that we are committing behind the stableTimestamp. + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); + + // Make sure we can see the insert from the prepared transaction. + arrayEq(sessionColl.find().toArray(), [{_id: 1}, {_id: 2}]); + + assert.commandWorked( + primary.adminCommand({configureFailPoint: 'WTSetOldestTSToStableTS', mode: 'off'})); + + replTest.stopSet(); +}());
\ No newline at end of file diff --git a/jstests/replsets/commit_transaction_recovery.js b/jstests/replsets/commit_transaction_recovery.js index 2e0ec6befe8..1f40a13938a 100644 --- a/jstests/replsets/commit_transaction_recovery.js +++ b/jstests/replsets/commit_transaction_recovery.js @@ -42,14 +42,13 @@ // Since the commitTimestamp is after the last snapshot, this oplog entry will be replayed // during replication recovery during restart. - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); jsTestLog("Restarting node"); // Perform a clean shutdown and restart. Note that the 'disableSnapshotting' failpoint will be // unset on the node following the restart. - replTest.stop(primary, undefined, {skipValidation: true}); - replTest.start(primary, {}, true); + replTest.restart(primary); jsTestLog("Node was restarted"); @@ -66,7 +65,7 @@ // Also, make sure that we can run another transaction after recovery without any problems. assert.commandWorked(sessionDB[collName].update({_id: 1}, {_id: 1, a: 1})); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 1}); replTest.stopSet(); diff --git a/jstests/replsets/commit_transaction_recovery_data_already_applied.js b/jstests/replsets/commit_transaction_recovery_data_already_applied.js index a6a28b6b726..7e8fd73f4a5 100644 --- a/jstests/replsets/commit_transaction_recovery_data_already_applied.js +++ b/jstests/replsets/commit_transaction_recovery_data_already_applied.js @@ -93,7 +93,7 @@ // Also, make sure that we can run another transaction after recovery without any problems. assert.commandWorked(sessionDB[collName].update({_id: 1}, {_id: 1, a: 1})); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 1}); replTest.stopSet(); diff --git a/jstests/replsets/commit_transaction_rollback_recovery_data_already_applied.js b/jstests/replsets/commit_transaction_rollback_recovery_data_already_applied.js index a31ad4b17d9..886fa36d501 100644 --- a/jstests/replsets/commit_transaction_rollback_recovery_data_already_applied.js +++ b/jstests/replsets/commit_transaction_rollback_recovery_data_already_applied.js @@ -88,7 +88,7 @@ // Also, make sure that we can run another transaction after recovery without any problems. assert.commandWorked(sessionDB[collName].update({_id: 1}, {_id: 1, a: 1})); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 1}); diff --git a/jstests/replsets/hang_before_releasing_transaction_oplog_hole.js b/jstests/replsets/hang_before_releasing_transaction_oplog_hole.js index 0ad75de1133..af74be61d60 100644 --- a/jstests/replsets/hang_before_releasing_transaction_oplog_hole.js +++ b/jstests/replsets/hang_before_releasing_transaction_oplog_hole.js @@ -45,7 +45,7 @@ assert.commandWorked(db.adminCommand( {configureFailPoint: 'hangBeforeReleasingTransactionOplogHole', mode: 'alwaysOn'})); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); } const joinTransaction = startParallelShell(transactionFn, rst.ports[0]); diff --git a/jstests/replsets/initial_sync_commit_prepared_transaction.js b/jstests/replsets/initial_sync_commit_prepared_transaction.js index 270fd3c3f4b..81590fc0bc2 100644 --- a/jstests/replsets/initial_sync_commit_prepared_transaction.js +++ b/jstests/replsets/initial_sync_commit_prepared_transaction.js @@ -77,8 +77,7 @@ // Commit a transaction on the sync source while collection cloning is paused so that we know // they must be applied during the oplog application stage of initial sync. - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp1)); + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp1)); jsTestLog("Resuming initial sync"); @@ -106,8 +105,7 @@ session2.startTransaction(); assert.commandWorked(sessionColl2.insert({_id: 4})); let prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session2, prepareTimestamp2)); + assert.commandWorked(PrepareHelpers.commitTransaction(session2, prepareTimestamp2)); res = newPrimary.getDB(dbName).getCollection(collName).findOne({_id: 4}); assert.docEq(res, {_id: 4}, res); diff --git a/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp.js b/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp.js index b5509f848c9..cece52cde31 100644 --- a/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp.js +++ b/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp.js @@ -63,9 +63,7 @@ // that since the beginApplyingTimestamp is the timestamp after which operations are applied // during initial sync, this commitTransaction will not be applied. const beginApplyingTimestamp = - assert - .commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp1)) + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp1)) .operationTime; jsTestLog("beginApplyingTimestamp: " + beginApplyingTimestamp); diff --git a/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp_no_oplog_application.js b/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp_no_oplog_application.js index 5d04f468f2c..fe0f0932ff1 100644 --- a/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp_no_oplog_application.js +++ b/jstests/replsets/initial_sync_fetch_from_oldest_active_transaction_timestamp_no_oplog_application.js @@ -68,9 +68,7 @@ // that since the beginApplyingTimestamp is the timestamp after which operations are applied // during initial sync, this commitTransaction will not be applied. const beginApplyingTimestamp = - assert - .commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp1)) + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp1)) .operationTime; jsTestLog("beginApplyingTimestamp/stopTimestamp: " + beginApplyingTimestamp); diff --git a/jstests/replsets/prepare_conflict_read_concern_behavior.js b/jstests/replsets/prepare_conflict_read_concern_behavior.js index 58efe922fa4..4327f7d86c5 100644 --- a/jstests/replsets/prepare_conflict_read_concern_behavior.js +++ b/jstests/replsets/prepare_conflict_read_concern_behavior.js @@ -69,13 +69,11 @@ assert.commandWorked(sessionColl.update({_id: 2}, {_id: 2, in_prepared_txn: true})); const prepareTimestamp = PrepareHelpers.prepareTransaction(session); - // TODO: Once we no longer hold the stable optime behind the earliest prepare optime - // whose corresponding commit/abort oplog entry optime is not majority committed, allow - // this insert to be majority committed. const clusterTimeAfterPrepare = assert - .commandWorked( - testColl.runCommand("insert", {documents: [{_id: 4, in_prepared_txn: false}]})) + .commandWorked(testColl.runCommand( + "insert", + {documents: [{_id: 4, in_prepared_txn: false}], writeConcern: {w: "majority"}})) .operationTime; jsTestLog("prepareTimestamp: " + prepareTimestamp + " clusterTimeBeforePrepare: " + @@ -85,12 +83,9 @@ assert.gt(prepareTimestamp, clusterTimeBeforePrepare); assert.gt(clusterTimeAfterPrepare, prepareTimestamp); - // TODO: Once we no longer hold the stable optime behind the earliest prepare optime - // whose corresponding commit/abort oplog entry optime is not majority committed, uncomment - // this read. - // jsTestLog( - // "Test read with read concern 'majority' doesn't block on a prepared transaction."); - // assert.commandWorked(read({level: 'majority'}, successTimeout, testDB, collName, 2)); + jsTestLog( + "Test read with read concern 'majority' doesn't block on a prepared transaction."); + assert.commandWorked(read({level: 'majority'}, successTimeout, testDB, collName, 3)); jsTestLog("Test read with read concern 'local' doesn't block on a prepared transaction."); assert.commandWorked(read({level: 'local'}, successTimeout, testDB, collName, 3)); @@ -147,13 +142,10 @@ session2.startTransaction( {readConcern: {level: "snapshot", atClusterTime: clusterTimeAfterPrepare}}); - // TODO: Once we no longer hold the stable optime behind the earliest prepare optime - // whose corresponding commit/abort oplog entry optime is not majority committed, uncomment - // this read. - // jsTestLog("Test read with read concern 'snapshot' and a read timestamp after " + - // "prepareTimestamp on non-prepared documents doesn't block on a prepared " + - // "transaction."); - // assert.commandWorked(read({}, failureTimeout, sessionDB2, collName2, 1)); + jsTestLog("Test read with read concern 'snapshot' and a read timestamp after " + + "prepareTimestamp on non-prepared documents doesn't block on a prepared " + + "transaction."); + assert.commandWorked(read({}, failureTimeout, sessionDB2, collName2, 1)); jsTestLog("Test read with read concern 'snapshot' and a read timestamp after " + "prepareTimestamp blocks on a prepared transaction."); diff --git a/jstests/replsets/prepare_transaction_index_build.js b/jstests/replsets/prepare_transaction_index_build.js index 3f657533a09..7f495ddceed 100644 --- a/jstests/replsets/prepare_transaction_index_build.js +++ b/jstests/replsets/prepare_transaction_index_build.js @@ -63,7 +63,7 @@ jsTestLog("Committing txn"); // Commit the transaction. - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); replTest.awaitReplication(); jsTestLog("Testing index integrity"); diff --git a/jstests/replsets/prepare_transaction_survives_state_transition_to_and_from_recovering.js b/jstests/replsets/prepare_transaction_survives_state_transition_to_and_from_recovering.js index d466ed296ea..fcab1ae7a27 100644 --- a/jstests/replsets/prepare_transaction_survives_state_transition_to_and_from_recovering.js +++ b/jstests/replsets/prepare_transaction_survives_state_transition_to_and_from_recovering.js @@ -52,8 +52,7 @@ jsTestLog("Commiting the second prepared transaction while a node is in the RECOVERING state"); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session2, prepareTimestamp2)); + assert.commandWorked(PrepareHelpers.commitTransaction(session2, prepareTimestamp2)); replSet.awaitReplication(); jsTestLog("Taking secondary out of maintenance mode so it will transition back to SECONDARY"); @@ -96,10 +95,9 @@ jsTestLog("Committing transaction"); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(newSession, prepareTimestamp1)); + assert.commandWorked(PrepareHelpers.commitTransaction(newSession, prepareTimestamp1)); replSet.awaitReplication(); replSet.stopSet(); -}());
\ No newline at end of file +}()); diff --git a/jstests/replsets/prepared_transaction_on_failover.js b/jstests/replsets/prepared_transaction_on_failover.js index 18e9fbefdc2..a87abbfc512 100644 --- a/jstests/replsets/prepared_transaction_on_failover.js +++ b/jstests/replsets/prepared_transaction_on_failover.js @@ -58,8 +58,7 @@ // Create a proxy session to reuse the session state of the old primary. const newSession = new _DelegatingDriverSession(newPrimary, session); - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(newSession, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(newSession, prepareTimestamp)); replTest.awaitReplication(); assert.docEq(doc, testDB.getCollection(collName).findOne()); diff --git a/jstests/replsets/recover_committed_aborted_prepared_transactions.js b/jstests/replsets/recover_committed_aborted_prepared_transactions.js index 4365b54ff1c..561e9e4eb2c 100644 --- a/jstests/replsets/recover_committed_aborted_prepared_transactions.js +++ b/jstests/replsets/recover_committed_aborted_prepared_transactions.js @@ -56,8 +56,7 @@ const prepareTimestamp2 = PrepareHelpers.prepareTransaction(session2); // Commit the first transaction. - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp)); // Abort the second transaction. session2.abortTransaction_forTesting(); @@ -112,8 +111,7 @@ assert.commandWorked(sessionColl1.insert({_id: 5})); const prepareTimestamp3 = PrepareHelpers.prepareTransaction(session1); // Make sure we can successfully retry the commitTransaction command after rollback. - assert.commandWorked( - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp3)); + assert.commandWorked(PrepareHelpers.commitTransaction(session1, prepareTimestamp3)); session1.startTransaction(); assert.commandWorked(sessionColl1.insert({_id: 6})); diff --git a/jstests/replsets/recover_multiple_prepared_transactions_startup.js b/jstests/replsets/recover_multiple_prepared_transactions_startup.js index 21781ce02e4..32fe036da15 100644 --- a/jstests/replsets/recover_multiple_prepared_transactions_startup.js +++ b/jstests/replsets/recover_multiple_prepared_transactions_startup.js @@ -152,7 +152,7 @@ session.startTransaction(); assert.commandWorked(sessionDB[collName].update({_id: 1}, {_id: 1, a: 3})); prepareTimestamp = PrepareHelpers.prepareTransaction(session); - assert.commandWorked(PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp)); + assert.commandWorked(PrepareHelpers.commitTransaction(session, prepareTimestamp)); assert.eq(testDB[collName].findOne({_id: 1}), {_id: 1, a: 3}); replTest.stopSet(); diff --git a/jstests/replsets/recover_prepared_transaction_state.js b/jstests/replsets/recover_prepared_transaction_state.js index fd6d630c819..e78e68169ec 100644 --- a/jstests/replsets/recover_prepared_transaction_state.js +++ b/jstests/replsets/recover_prepared_transaction_state.js @@ -77,7 +77,7 @@ // The following commit and abort will be rolled back. rollbackTest.transitionToRollbackOperations(); - PrepareHelpers.commitTransactionAfterPrepareTS(session1, prepareTimestamp); + PrepareHelpers.commitTransaction(session1, prepareTimestamp); session2.abortTransaction_forTesting(); // The fastcount should be accurate because there are no open transactions. diff --git a/jstests/replsets/rollback_aborted_prepared_transaction.js b/jstests/replsets/rollback_aborted_prepared_transaction.js index bd7e3057105..3093582aede 100644 --- a/jstests/replsets/rollback_aborted_prepared_transaction.js +++ b/jstests/replsets/rollback_aborted_prepared_transaction.js @@ -79,7 +79,7 @@ session.startTransaction(); assert.commandWorked(sessionColl.insert({_id: 1})); const prepareTimestamp = PrepareHelpers.prepareTransaction(session); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); assert.eq(testColl.find().itcount(), 2); assert.eq(testColl.count(), 2); diff --git a/jstests/replsets/rollback_via_refetch_commit_transaction.js b/jstests/replsets/rollback_via_refetch_commit_transaction.js index 8f51dcdfedf..bb4ca534553 100644 --- a/jstests/replsets/rollback_via_refetch_commit_transaction.js +++ b/jstests/replsets/rollback_via_refetch_commit_transaction.js @@ -50,7 +50,7 @@ TestData.skipCheckDBHashes = true; session.getDatabase('admin').adminCommand({prepareTransaction: 1, writeConcern: {w: 1}})); assert(result.prepareTimestamp, "prepareTransaction did not return a 'prepareTimestamp': " + tojson(result)); - PrepareHelpers.commitTransactionAfterPrepareTS(session, result.prepareTimestamp); + PrepareHelpers.commitTransaction(session, result.prepareTimestamp); // Step down current primary and elect a node that lacks the commit. rollbackTest.transitionToSyncSourceOperationsBeforeRollback(); diff --git a/jstests/replsets/rollover_preserves_active_txns.js b/jstests/replsets/rollover_preserves_active_txns.js index e6743c5f25a..8597d1aa3cc 100644 --- a/jstests/replsets/rollover_preserves_active_txns.js +++ b/jstests/replsets/rollover_preserves_active_txns.js @@ -83,7 +83,7 @@ if (commitOrAbort === "commit") { jsTestLog("Commit prepared transaction and wait for oplog to shrink to max oplogSize"); - PrepareHelpers.commitTransactionAfterPrepareTS(session, prepareTimestamp); + PrepareHelpers.commitTransaction(session, prepareTimestamp); } else if (commitOrAbort === "abort") { jsTestLog("Abort prepared transaction and wait for oplog to shrink to max oplogSize"); session.abortTransaction_forTesting(); diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 674e244acd8..90e8fe71c73 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -74,7 +74,6 @@ #include "mongo/db/repl/update_position_args.h" #include "mongo/db/repl/vote_requester.h" #include "mongo/db/server_options.h" -#include "mongo/db/server_transactions_metrics.h" #include "mongo/db/transaction_participant.h" #include "mongo/db/write_concern.h" #include "mongo/db/write_concern_options.h" @@ -3376,22 +3375,6 @@ boost::optional<OpTime> ReplicationCoordinatorImpl::_chooseStableOpTimeFromCandi maximumStableOpTime = OpTime(maximumStableTimestamp, maximumStableOpTime.getTerm()); - // When calculating the stable optime, compare it to the oldest oplog entry timestamp across - // transactions whose corresponding commit/abort oplog entries have not been majority committed. - const auto serverTxnMetrics = ServerTransactionsMetrics::get(getGlobalServiceContext()); - const auto oldestNonMajCommittedOpTime = - serverTxnMetrics->getOldestNonMajorityCommittedOpTime(); - - if (oldestNonMajCommittedOpTime) { - if (oldestNonMajCommittedOpTime->getTimestamp() < maximumStableTimestamp) { - // If there is an oldest non-majority committed timestamp that is less than the current - // max stable timestamp, then update the max stable timestamp/optime accordingly. - maximumStableTimestamp = oldestNonMajCommittedOpTime->getTimestamp(); - maximumStableOpTime = - OpTime(maximumStableTimestamp, oldestNonMajCommittedOpTime->getTerm()); - } - } - // Find the greatest optime candidate that is less than or equal to 'maximumStableOpTime'. To do // this we first find the upper bound of 'maximumStableOpTime', which points to the smallest // element in 'candidates' that is greater than 'maximumStableOpTime'. We then step back one @@ -3451,16 +3434,6 @@ boost::optional<OpTime> ReplicationCoordinatorImpl::_recalculateStableOpTime(Wit invariant(snapshotOpTime <= commitPoint); } - // If we advanced the commit point and have prepared transactions, check if their commit or - // abort timestamps are <= the commit point. If so, remove them from our oldest non-majority - // committed optimes set because we know that the commit/abort oplog entries are majority - // committed. - // We must remove these optimes before calling _chooseStableOpTimeFromCandidates - // because we want the stable timestamp to advance up to the commit point if all transactions - // are committed or aborted. - auto txnMetrics = ServerTransactionsMetrics::get(getGlobalServiceContext()); - txnMetrics->removeOpTimesLessThanOrEqToCommittedOpTime(commitPoint); - // When majority read concern is disabled, the stable opTime is set to the lastApplied, rather // than the commit point. auto maximumStableOpTime = serverGlobalParams.enableMajorityReadConcern diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp index ff89cc2257c..a22ffa49567 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp @@ -58,7 +58,6 @@ #include "mongo/db/repl/topology_coordinator.h" #include "mongo/db/repl/update_position_args.h" #include "mongo/db/server_options.h" -#include "mongo/db/server_transactions_metrics.h" #include "mongo/db/service_context.h" #include "mongo/db/write_concern_options.h" #include "mongo/executor/network_interface_mock.h" @@ -3864,8 +3863,6 @@ TEST_F(StableOpTimeTest, CalculateStableOpTime) { initReplSetMode(); auto repl = getReplCoord(); - getStorageInterface()->supportsDocLockingBool = true; - auto txnMetrics = ServerTransactionsMetrics::get(getGlobalServiceContext()); OpTime commitPoint; boost::optional<OpTime> expectedStableOpTime, stableOpTime; std::set<OpTime> stableOpTimeCandidates; @@ -3936,23 +3933,6 @@ TEST_F(StableOpTimeTest, CalculateStableOpTime) { stableOpTime = repl->chooseStableOpTimeFromCandidates_forTest(stableOpTimeCandidates, commitPoint); ASSERT_EQ(expectedStableOpTime, stableOpTime); - - // Set the oldest oplog entry OpTime for non-majority committed aborts/commits associated - // with a multi-document transaction to be before the current commit point. We will then - // make expectedStableOpTime the new stable optime because it is the only candidate before - // the oldestNonMajCommittedOpTime. - commitPoint = OpTime({1, 5}, term); - const auto oldestNonMajCommittedOpTime = OpTime({1, 3}, term); - const auto finishOpTime = OpTime({1, 3}, term); - stableOpTimeCandidates = {oldestNonMajCommittedOpTime, OpTime({1, 4}, term)}; - - // Adds the oldestNonMajCommittedOpTime to both sets. - txnMetrics->addActiveOpTime(oldestNonMajCommittedOpTime); - // Update the finishOpTime. - txnMetrics->removeActiveOpTime(oldestNonMajCommittedOpTime, finishOpTime); - stableOpTime = - repl->chooseStableOpTimeFromCandidates_forTest(stableOpTimeCandidates, commitPoint); - ASSERT_EQ(oldestNonMajCommittedOpTime, stableOpTime); } TEST_F(StableOpTimeTest, CleanupStableOpTimeCandidates) { diff --git a/src/mongo/db/repl/rollback_impl_test.cpp b/src/mongo/db/repl/rollback_impl_test.cpp index 354f9f7f3dd..1fcc2bea878 100644 --- a/src/mongo/db/repl/rollback_impl_test.cpp +++ b/src/mongo/db/repl/rollback_impl_test.cpp @@ -1387,15 +1387,12 @@ TEST_F(RollbackImplTest, RollbackCallsClearOpTimes) { ASSERT(txnMetrics->getOldestActiveOpTime()); ASSERT_EQ(*txnMetrics->getOldestActiveOpTime(), repl::OpTime(Timestamp(1, 2), 0)); ASSERT_EQ(txnMetrics->getTotalActiveOpTimes(), 2U); - ASSERT(txnMetrics->getOldestNonMajorityCommittedOpTime()); - ASSERT_EQ(*txnMetrics->getOldestNonMajorityCommittedOpTime(), repl::OpTime(Timestamp(1, 2), 0)); // Call runRollback to make sure these variables get cleared. ASSERT_OK(_rollback->runRollback(_opCtx.get())); ASSERT_FALSE(txnMetrics->getOldestActiveOpTime()); ASSERT_EQ(txnMetrics->getTotalActiveOpTimes(), 0U); - ASSERT_FALSE(txnMetrics->getOldestNonMajorityCommittedOpTime()); } /** diff --git a/src/mongo/db/server_transactions_metrics.cpp b/src/mongo/db/server_transactions_metrics.cpp index a46a663e0b1..5696b7079be 100644 --- a/src/mongo/db/server_transactions_metrics.cpp +++ b/src/mongo/db/server_transactions_metrics.cpp @@ -149,8 +149,9 @@ void ServerTransactionsMetrics::decrementCurrentPrepared() { _currentPrepared.fetchAndSubtract(1); } -boost::optional<repl::OpTime> ServerTransactionsMetrics::_calculateOldestActiveOpTime( - WithLock) const { + +boost::optional<repl::OpTime> ServerTransactionsMetrics::getOldestActiveOpTime() const { + stdx::lock_guard<stdx::mutex> lm(_mutex); if (_oldestActiveOplogEntryOpTimes.empty()) { return boost::none; } @@ -162,110 +163,20 @@ void ServerTransactionsMetrics::addActiveOpTime(repl::OpTime oldestOplogEntryOpT auto ret = _oldestActiveOplogEntryOpTimes.insert(oldestOplogEntryOpTime); // If ret.second is false, the OpTime we tried to insert already existed. invariant(ret.second, - str::stream() << "This oplog entry OpTime already exists in " - << "oldestActiveOplogEntryOpTimes." + str::stream() << "This oplog entry OpTime already exists." << "oldestOplogEntryOpTime: " << oldestOplogEntryOpTime.toString()); - - // Add this OpTime to the oldestNonMajorityCommittedOpTimes set with a finishOpTime of - // Timestamp::max() to signify that it has not been committed/aborted. - std::pair<repl::OpTime, repl::OpTime> nonMajCommittedOpTime(oldestOplogEntryOpTime, - repl::OpTime::max()); - auto ret2 = _oldestNonMajorityCommittedOpTimes.insert(nonMajCommittedOpTime); - // If ret2.second is false, the OpTime we tried to insert already existed. - invariant(ret2.second, - str::stream() << "This oplog entry OpTime already exists in " - << "oldestNonMajorityCommittedOpTimes." - << "oldestOplogEntryOpTime: " - << oldestOplogEntryOpTime.toString()); - _oldestActiveOplogEntryOpTime = _calculateOldestActiveOpTime(lm); } -void ServerTransactionsMetrics::removeActiveOpTime(repl::OpTime oldestOplogEntryOpTime, - boost::optional<repl::OpTime> finishOpTime) { +void ServerTransactionsMetrics::removeActiveOpTime(repl::OpTime oldestOplogEntryOpTime) { stdx::lock_guard<stdx::mutex> lm(_mutex); auto it = _oldestActiveOplogEntryOpTimes.find(oldestOplogEntryOpTime); invariant(it != _oldestActiveOplogEntryOpTimes.end(), - str::stream() << "This oplog entry OpTime does not exist in or has already been " - << "removed from oldestActiveOplogEntryOpTimes." - << "OpTime: " - << oldestOplogEntryOpTime.toString()); - _oldestActiveOplogEntryOpTimes.erase(it); - - if (!finishOpTime) { - return; - } - - // The transaction's oldestOplogEntryOpTime now has a corresponding finishTime, which will - // be its commit or abort oplog entry OpTime. Add this pair to the - // oldestNonMajorityCommittedOpTimes. - // Currently, the oldestOplogEntryOpTime will be a prepareOpTime so we will only have a - // finishOpTime if we are committing a prepared transaction or aborting an active prepared - // transaction. - std::pair<repl::OpTime, repl::OpTime> opTimeToRemove(oldestOplogEntryOpTime, - repl::OpTime::max()); - auto it2 = _oldestNonMajorityCommittedOpTimes.find(opTimeToRemove); - invariant(it2 != _oldestNonMajorityCommittedOpTimes.end(), - str::stream() << "This oplog entry OpTime does not exist in or has already been " - << "removed from oldestNonMajorityCommittedOpTimes" + str::stream() << "This oplog entry OpTime does not exist " + << "or has already been removed." << "oldestOplogEntryOpTime: " << oldestOplogEntryOpTime.toString()); - _oldestNonMajorityCommittedOpTimes.erase(it2); - - std::pair<repl::OpTime, repl::OpTime> nonMajCommittedOpTime(oldestOplogEntryOpTime, - *finishOpTime); - auto ret = _oldestNonMajorityCommittedOpTimes.insert(nonMajCommittedOpTime); - // If ret.second is false, the OpTime we tried to insert already existed. - invariant(ret.second, - str::stream() << "This oplog entry OpTime pair already exists in " - << "oldestNonMajorityCommittedOpTimes." - << "oldestOplogEntryOpTime: " - << oldestOplogEntryOpTime.toString() - << "finishOpTime: " - << finishOpTime->toString()); - _oldestActiveOplogEntryOpTime = _calculateOldestActiveOpTime(lm); -} - -boost::optional<repl::OpTime> ServerTransactionsMetrics::getOldestNonMajorityCommittedOpTime() - const { - stdx::lock_guard<stdx::mutex> lm(_mutex); - if (_oldestNonMajorityCommittedOpTimes.empty()) { - return boost::none; - } - const auto oldestNonMajorityCommittedOpTime = _oldestNonMajorityCommittedOpTimes.begin()->first; - invariant(!oldestNonMajorityCommittedOpTime.isNull()); - return oldestNonMajorityCommittedOpTime; -} - -void ServerTransactionsMetrics::removeOpTimesLessThanOrEqToCommittedOpTime( - repl::OpTime committedOpTime) { - stdx::lock_guard<stdx::mutex> lm(_mutex); - // Iterate through oldestNonMajorityCommittedOpTimes and remove all pairs whose "finishOpTime" - // is now less than or equal to the commit point. - for (auto it = _oldestNonMajorityCommittedOpTimes.begin(); - it != _oldestNonMajorityCommittedOpTimes.end();) { - if (it->second <= committedOpTime) { - it = _oldestNonMajorityCommittedOpTimes.erase(it); - } else { - ++it; - } - } -} - -boost::optional<repl::OpTime> -ServerTransactionsMetrics::getFinishOpTimeOfOldestNonMajCommitted_forTest() const { - stdx::lock_guard<stdx::mutex> lm(_mutex); - if (_oldestNonMajorityCommittedOpTimes.empty()) { - return boost::none; - } - const auto oldestNonMajorityCommittedOpTime = - _oldestNonMajorityCommittedOpTimes.begin()->second; - return oldestNonMajorityCommittedOpTime; -} - -boost::optional<repl::OpTime> ServerTransactionsMetrics::getOldestActiveOpTime() const { - stdx::lock_guard<stdx::mutex> lm(_mutex); - return _oldestActiveOplogEntryOpTime; + _oldestActiveOplogEntryOpTimes.erase(it); } unsigned int ServerTransactionsMetrics::getTotalActiveOpTimes() const { @@ -294,21 +205,17 @@ void ServerTransactionsMetrics::updateStats(TransactionsStats* stats, OperationC stats->setCurrentPrepared(_currentPrepared.load()); stats->setOldestOpenUnpreparedReadTimestamp( ServerTransactionsMetrics::_getOldestOpenUnpreparedReadTimestamp(opCtx)); - // Acquire _mutex before reading _oldestActiveOplogEntryOpTime. - stdx::lock_guard<stdx::mutex> lm(_mutex); - // To avoid compression loss, we use the null OpTime if no oldest active transaction optime is - // stored. - repl::OpTime oldestActiveOplogEntryOpTime = (_oldestActiveOplogEntryOpTime != boost::none) - ? _oldestActiveOplogEntryOpTime.get() - : repl::OpTime(); + // To avoid compression loss, we use the null OpTime if no oldest active transaction + // optime is stored. + auto oldestActiveOpTime = getOldestActiveOpTime(); + repl::OpTime oldestActiveOplogEntryOpTime = + (oldestActiveOpTime != boost::none) ? oldestActiveOpTime.get() : repl::OpTime(); stats->setOldestActiveOplogEntryOpTime(oldestActiveOplogEntryOpTime); } void ServerTransactionsMetrics::clearOpTimes() { stdx::lock_guard<stdx::mutex> lm(_mutex); - _oldestActiveOplogEntryOpTime = boost::none; _oldestActiveOplogEntryOpTimes.clear(); - _oldestNonMajorityCommittedOpTimes.clear(); } namespace { diff --git a/src/mongo/db/server_transactions_metrics.h b/src/mongo/db/server_transactions_metrics.h index 035e9ad6ee0..706200409da 100644 --- a/src/mongo/db/server_transactions_metrics.h +++ b/src/mongo/db/server_transactions_metrics.h @@ -31,6 +31,7 @@ #include <set> +#include "mongo/bson/timestamp.h" #include "mongo/db/operation_context.h" #include "mongo/db/repl/optime.h" #include "mongo/db/service_context.h" @@ -93,20 +94,15 @@ public: boost::optional<repl::OpTime> getOldestActiveOpTime() const; /** - * Add the transaction's oplog entry OpTime to oldestActiveOplogEntryOpTimes, a set of OpTimes. - * Also creates a pair with this OpTime and OpTime::max() as the corresponding commit/abort - * oplog entry OpTime. Finally, adds this to oldestNonMajorityCommittedOpTimes. + * Add the transaction's oplog entry OpTime to a set of OpTimes. */ void addActiveOpTime(repl::OpTime oldestOplogEntryOpTime); /** * Remove the corresponding transaction oplog entry OpTime if the transaction commits or - * aborts. Also updates the pair in oldestNonMajorityCommittedOpTimes with the - * oldestOplogEntryOpTime to have a valid finishOpTime instead of OpTime::max(). It's stored in - * the format: < oldestOplogEntryOpTime, finishOpTime >. + * aborts. */ - void removeActiveOpTime(repl::OpTime oldestOplogEntryOpTime, - boost::optional<repl::OpTime> finishOpTime); + void removeActiveOpTime(repl::OpTime oldestOplogEntryOpTime); /** * Returns the number of transaction oplog entry OpTimes currently stored. @@ -114,51 +110,19 @@ public: unsigned int getTotalActiveOpTimes() const; /** - * Returns the oldest oplog entry OpTime across transactions whose corresponding commit or - * abort oplog entry has not been majority committed. - */ - boost::optional<repl::OpTime> getOldestNonMajorityCommittedOpTime() const; - - /** - * Remove the corresponding transaction oplog entry OpTime pair from - * oldestNonMajorityCommittedOpTimes if the transaction is majority committed or aborted. - * We determine this by checking if there are any pairs in the set whose - * 'finishOpTime' <= 'committedOpTime'. - */ - void removeOpTimesLessThanOrEqToCommittedOpTime(repl::OpTime committedOpTime); - - /** - * Testing function that adds an OpTime pair to oldestNonMajorityCommittedOpTimes. - */ - void addNonMajCommittedOpTimePair_forTest(std::pair<repl::OpTime, repl::OpTime> OpTimePair); - - /** - * Testing function that returns the oldest non-majority committed OpTime pair in the form: - * < oldestOplogEntryOpTime, finishOpTime >. - */ - boost::optional<repl::OpTime> getFinishOpTimeOfOldestNonMajCommitted_forTest() const; - - /** * Appends the accumulated stats to a transactions stats object. */ void updateStats(TransactionsStats* stats, OperationContext* opCtx); /** * Invalidates the in-memory state of prepared transactions during replication rollback by - * clearing oldestActiveOplogEntryOpTime, oldestActiveOplogEntryOpTimes, and - * oldestNonMajorityCommittedOpTimes. These variables/data structures should be properly - * reconstructed during replication recovery. + * clearing _oldestActiveOplogEntryOpTimes. This data structure should be properly reconstructed + * during replication recovery. */ void clearOpTimes(); private: /** - * Returns the first and oldest optime in the ordered set of active oplog entry optimes. - * Returns boost::none if there are no transaction oplog entry optimes stored. - */ - boost::optional<repl::OpTime> _calculateOldestActiveOpTime(WithLock) const; - - /** * Returns the oldest read timestamp in use by any open unprepared transaction. This will * return a null timestamp if there is no oldest open unprepared read timestamp to be * returned. @@ -203,24 +167,10 @@ private: // The current number of transactions in the prepared state. AtomicWord<unsigned long long> _currentPrepared{0}; - // The optime of the oldest oplog entry for any active transaction. - boost::optional<repl::OpTime> _oldestActiveOplogEntryOpTime; // (M) - // Maintain the oldest oplog entry OpTime across all active transactions. Currently, we only // write an oplog entry for an ongoing transaction if it is in the `prepare` state. By // maintaining an ordered set of OpTimes, the OpTime at the beginning will be the oldest. std::set<repl::OpTime> _oldestActiveOplogEntryOpTimes; // (M) - - // Maintain the oldest oplog entry OpTime across transactions whose corresponding abort/commit - // oplog entries have not been majority committed. Since this is an ordered set, the first - // pair's oldestOplogEntryOpTime represents the earliest OpTime that we should pin the stable - // timestamp behind. - // Each pair is structured as follows: <oldestOplogEntryOpTime, finishOpTime> - // 'oldestOplogEntryOpTime': The first oplog entry OpTime written by a transaction. - // 'finishOpTime': The commit/abort oplog entry OpTime. - // Once the corresponding abort/commit entry has been majority committed, remove the pair from - // this set. - std::set<std::pair<repl::OpTime, repl::OpTime>> _oldestNonMajorityCommittedOpTimes; // (M) }; } // namespace mongo diff --git a/src/mongo/db/transaction_metrics_observer.cpp b/src/mongo/db/transaction_metrics_observer.cpp index e1f092d60b5..fd19a6cba65 100644 --- a/src/mongo/db/transaction_metrics_observer.cpp +++ b/src/mongo/db/transaction_metrics_observer.cpp @@ -95,11 +95,8 @@ void TransactionMetricsObserver::onUnstash(ServerTransactionsMetrics* serverTran void TransactionMetricsObserver::onCommit(ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> commitOpTime, Top* top, bool wasPrepared) { - invariant((oldestOplogEntryOpTime != boost::none && commitOpTime != boost::none) || - (oldestOplogEntryOpTime == boost::none && commitOpTime == boost::none)); // // Per transaction metrics. // @@ -129,7 +126,7 @@ void TransactionMetricsObserver::onCommit(ServerTransactionsMetrics* serverTrans // Remove this transaction's oldest oplog entry OpTime if one was written. if (oldestOplogEntryOpTime) { - serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime, commitOpTime); + serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime); } } @@ -137,11 +134,8 @@ void TransactionMetricsObserver::_onAbortActive( ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> abortOpTime, Top* top, bool wasPrepared) { - invariant((oldestOplogEntryOpTime != boost::none && abortOpTime != boost::none) || - (oldestOplogEntryOpTime == boost::none && abortOpTime == boost::none)); auto curTick = tickSource->getTicks(); invariant(_singleTransactionStats.isActive()); @@ -163,7 +157,7 @@ void TransactionMetricsObserver::_onAbortActive( // Remove this transaction's oldest oplog entry OpTime if one was written. if (oldestOplogEntryOpTime) { - serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime, abortOpTime); + serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime); } } @@ -183,25 +177,22 @@ void TransactionMetricsObserver::_onAbortInactive( // Remove this transaction's oldest oplog entry OpTime if one was written. if (oldestOplogEntryOpTime) { - serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime, boost::none); + serverTransactionsMetrics->removeActiveOpTime(*oldestOplogEntryOpTime); } } void TransactionMetricsObserver::onAbort(ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> abortOpTime, Top* top, bool wasPrepared) { if (_singleTransactionStats.isActive()) { _onAbortActive(serverTransactionsMetrics, tickSource, oldestOplogEntryOpTime, - abortOpTime, top, wasPrepared); } else { - invariant(abortOpTime == boost::none); invariant(!wasPrepared); _onAbortInactive(serverTransactionsMetrics, tickSource, oldestOplogEntryOpTime, top); } @@ -260,14 +251,13 @@ void TransactionMetricsObserver::_onAbort(ServerTransactionsMetrics* serverTrans void TransactionMetricsObserver::onPrepare(ServerTransactionsMetrics* serverTransactionsMetrics, repl::OpTime prepareOpTime, TickSource::Tick curTick) { - // // Per transaction metrics. // _singleTransactionStats.setPreparedStartTime(curTick); // Since we currently only write an oplog entry for an in progress transaction when it is in - // the prepare state, the prepareOpTime is currently the oldest OpTime written to the + // the prepare state, the prepareOpTime is currently the oldest optime written to the // oplog for this transaction. serverTransactionsMetrics->addActiveOpTime(prepareOpTime); serverTransactionsMetrics->incrementCurrentPrepared(); diff --git a/src/mongo/db/transaction_metrics_observer.h b/src/mongo/db/transaction_metrics_observer.h index cbb00d21fd9..2634e56728f 100644 --- a/src/mongo/db/transaction_metrics_observer.h +++ b/src/mongo/db/transaction_metrics_observer.h @@ -70,12 +70,10 @@ public: /** * Updates relevant metrics when a transaction commits. Also removes this transaction's oldest * oplog entry OpTime from the oldestActiveOplogEntryOpTimes set if it is not boost::none. - * Finally, updates an entry in oldestNonMajorityCommittedOpTimes to include its commit OpTime. */ void onCommit(ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> commitOpTime, Top* top, bool wasPrepared); @@ -86,13 +84,12 @@ public: void onAbort(ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> abortOpTime, Top* top, bool wasPrepared); /** - * Adds the prepareOpTime, which is currently the Timestamp of the first oplog entry written by - * an active transaction, to the oldestActiveOplogEntryTS set. + * Adds the prepareOpTime, which is currently the OpTime of the first oplog entry written + * by an active transaction, to the oldestActiveOplogEntryOpTimes set. */ void onPrepare(ServerTransactionsMetrics* serverTransactionsMetrics, repl::OpTime prepareOpTime, @@ -138,7 +135,6 @@ private: void _onAbortActive(ServerTransactionsMetrics* serverTransactionsMetrics, TickSource* tickSource, boost::optional<repl::OpTime> oldestOplogEntryOpTime, - boost::optional<repl::OpTime> abortOpTime, Top* top, bool wasPrepared); diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp index 8ed304c50d7..59ec112a210 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -1030,14 +1030,11 @@ Timestamp TransactionParticipant::Participant::prepareTransaction( abortGuard.dismiss(); - // For prepared transactions, we must update ServerTransactionMetrics with the prepare optime - // before the prepare oplog entry is written so that we don't incorrectly advance the stable - // timestamp. invariant(!p().oldestOplogEntryOpTime, - str::stream() << "This transaction's oldest oplog entry Timestamp has already " + str::stream() << "This transaction's oldest oplog entry OpTime has already " << "been set to: " << p().oldestOplogEntryOpTime->toString()); - // Keep track of the Timestamp from the first oplog entry written by this transaction. + // Keep track of the OpTime from the first oplog entry written by this transaction. p().oldestOplogEntryOpTime = prepareOplogSlot.opTime; // Maintain the OpTime of the oldest active oplog entry for this transaction. We currently @@ -1126,7 +1123,7 @@ void TransactionParticipant::Participant::commitUnpreparedTransaction(OperationC // TODO SERVER-37129: Remove this invariant once we allow transactions larger than 16MB. invariant(!p().oldestOplogEntryOpTime, - str::stream() << "The oldest oplog entry Timestamp should not have been set because " + str::stream() << "The oldest oplog entry OpTime should not have been set because " << "this transaction is not prepared. But, it is currently " << p().oldestOplogEntryOpTime->toString()); @@ -1191,8 +1188,8 @@ void TransactionParticipant::Participant::commitPreparedTransaction( uassert( ErrorCodes::InvalidOptions, "'commitTimestamp' cannot be null", !commitTimestamp.isNull()); uassert(ErrorCodes::InvalidOptions, - "'commitTimestamp' must be greater than the 'prepareTimestamp'", - commitTimestamp > o().prepareOpTime.getTimestamp()); + "'commitTimestamp' must be greater than or equal to 'prepareTimestamp'", + commitTimestamp >= o().prepareOpTime.getTimestamp()); { stdx::lock_guard<Client> lk(*opCtx->getClient()); @@ -1245,9 +1242,8 @@ void TransactionParticipant::Participant::commitPreparedTransaction( clearOperationsInMemory(opCtx); // If we are committing a prepared transaction, then we must have already recorded this - // transaction's oldest oplog entry optime. + // transaction's oldest oplog entry OpTime. invariant(p().oldestOplogEntryOpTime); - p().finishOpTime = commitOplogSlotOpTime; _finishCommitTransaction(opCtx); } catch (...) { @@ -1300,7 +1296,6 @@ void TransactionParticipant::Participant::_finishCommitTransaction(OperationCont o(lk).transactionMetricsObserver.onCommit(ServerTransactionsMetrics::get(opCtx), tickSource, p().oldestOplogEntryOpTime, - p().finishOpTime, &Top::get(getGlobalServiceContext()), isCommittingWithPrepare); o(lk).transactionMetricsObserver.onTransactionOperation( @@ -1364,7 +1359,7 @@ void TransactionParticipant::Participant::abortActiveUnpreparedOrStashPreparedTr // TODO SERVER-37129: Remove this invariant once we allow transactions larger than 16MB. invariant(!p().oldestOplogEntryOpTime, - str::stream() << "The oldest oplog entry Timestamp should not have been set because " + str::stream() << "The oldest oplog entry OpTime should not have been set because " << "this transaction is not prepared. But, it is currently " << p().oldestOplogEntryOpTime->toString()); @@ -1409,12 +1404,6 @@ void TransactionParticipant::Participant::_abortActiveTransaction( invariant(opObserver); opObserver->onTransactionAbort(opCtx, abortOplogSlot); - // Set the finishOpTime of this transaction if we have recorded this transaction's oldest oplog - // entry optime. - if (p().oldestOplogEntryOpTime) { - p().finishOpTime = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp(); - } - // Only abort the transaction in session if it's in expected states. // When the state of active transaction on session is not expected, it means another // thread has already aborted the transaction on session. @@ -1454,7 +1443,6 @@ void TransactionParticipant::Participant::_abortTransactionOnSession(OperationCo ServerTransactionsMetrics::get(opCtx->getServiceContext()), tickSource, p().oldestOplogEntryOpTime, - p().finishOpTime, &Top::get(opCtx->getServiceContext()), o().txnState.isPrepared()); } @@ -1772,10 +1760,6 @@ std::string TransactionParticipant::Participant::_transactionInfoForLog( s << " oldestOplogEntryOpTime:" << p().oldestOplogEntryOpTime->toString(); } - if (p().finishOpTime) { - s << " finishOpTime:" << p().finishOpTime->toString(); - } - // Total duration of the transaction. s << ", " << duration_cast<Milliseconds>(singleTransactionStats.getDuration(tickSource, curTick)); @@ -1949,7 +1933,6 @@ void TransactionParticipant::Participant::_resetTransactionState( p().transactionOperations.clear(); o(wl).prepareOpTime = repl::OpTime(); p().oldestOplogEntryOpTime = boost::none; - p().finishOpTime = boost::none; p().speculativeTransactionReadOpTime = repl::OpTime(); p().multikeyPathInfo.clear(); p().autoCommit = boost::none; @@ -1989,8 +1972,8 @@ void TransactionParticipant::Participant::abortPreparedTransactionForRollback( // we only modify these variables when adding an operation to a transaction. Since this // transaction is already prepared, we cannot add more operations to it. We will have this // in the prepare oplog entry. - // Both _finishOpTime and _oldestOplogEntryOpTime will be reset to boost::none. With a - // prepared transaction, the latter is the same as the prepareOpTime. + // The oldestOplogEntryOpTime will be reset to boost::none. With a prepared transaction, this + // is the same as the prepareOpTime. _resetTransactionState(lg, TransactionState::kNone); } diff --git a/src/mongo/db/transaction_participant.h b/src/mongo/db/transaction_participant.h index e68c3205afd..6d5b1718513 100644 --- a/src/mongo/db/transaction_participant.h +++ b/src/mongo/db/transaction_participant.h @@ -633,10 +633,6 @@ public: return p().oldestOplogEntryOpTime; } - boost::optional<repl::OpTime> getFinishOpTimeForTest() const { - return p().finishOpTime; - } - const Locker* getTxnResourceStashLockerForTest() const { invariant(o().txnResourceStash); return o().txnResourceStash->locker(); @@ -916,9 +912,6 @@ private: // Tracks the OpTime of the first oplog entry written by this TransactionParticipant. boost::optional<repl::OpTime> oldestOplogEntryOpTime; - // Tracks the OpTime of the abort/commit oplog entry associated with this transaction. - boost::optional<repl::OpTime> finishOpTime; - // // Retryable writes state // diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp index 5e4051da203..31d09c22656 100644 --- a/src/mongo/db/transaction_participant_test.cpp +++ b/src/mongo/db/transaction_participant_test.cpp @@ -41,7 +41,6 @@ #include "mongo/db/repl/oplog.h" #include "mongo/db/repl/oplog_entry.h" #include "mongo/db/repl/optime.h" -#include "mongo/db/repl/repl_client_info.h" #include "mongo/db/server_transactions_metrics.h" #include "mongo/db/service_context.h" #include "mongo/db/session_catalog_mongod.h" @@ -632,21 +631,6 @@ TEST_F(TxnParticipantTest, ErrorCodes::InvalidOptions); } -TEST_F(TxnParticipantTest, - CommitTransactionWithCommitTimestampEqualToPrepareTimestampFailsOnPreparedTransaction) { - auto sessionCheckout = checkOutSession(); - auto txnParticipant = TransactionParticipant::get(opCtx()); - - txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); - - // The transaction machinery cannot store an empty locker. - Lock::GlobalLock lk(opCtx(), MODE_IX, Date_t::now(), Lock::InterruptBehavior::kThrow); - auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - ASSERT_THROWS_CODE(txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}), - AssertionException, - ErrorCodes::InvalidOptions); -} - // This test makes sure the abort machinery works even when no operations are done on the // transaction. TEST_F(TxnParticipantTest, EmptyTransactionAbort) { @@ -745,7 +729,6 @@ TEST_F(TxnParticipantTest, StepDownAfterPrepareDoesNotBlockThenCommit) { txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTS = Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); // Test that we can acquire the RSTL in mode X, and then immediately release it so the test can // complete successfully. @@ -755,7 +738,7 @@ TEST_F(TxnParticipantTest, StepDownAfterPrepareDoesNotBlockThenCommit) { }; runFunctionFromDifferentOpCtx(func); - txnParticipant.commitPreparedTransaction(opCtx(), commitTS, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); ASSERT(_opObserver->preparedTransactionCommitted); ASSERT(txnParticipant.transactionIsCommitted()); } @@ -1024,9 +1007,9 @@ DEATH_TEST_F(TxnParticipantTest, txnParticipant.unstashTransactionResources(opCtx(), "insert"); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); + txnParticipant.addTransactionOperation(opCtx(), operation); auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - auto commitTS = Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); // Check that the oldest prepareTimestamp is the one we just set. auto prepareOpTime = ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime(); @@ -1041,9 +1024,10 @@ DEATH_TEST_F(TxnParticipantTest, ASSERT_FALSE(txnParticipant.transactionIsAborted()); }; - txnParticipant.commitPreparedTransaction(opCtx(), commitTS, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); // Check that we removed the prepareTimestamp from the set. - ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime()); + auto oldestActiveTS = ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime(); + ASSERT_FALSE(oldestActiveTS); } TEST_F(TxnParticipantTest, CannotContinueNonExistentTransaction) { @@ -1462,15 +1446,14 @@ TEST_F(TransactionsMetricsTest, IncrementTotalCommittedOnCommit) { TEST_F(TransactionsMetricsTest, IncrementTotalPreparedThenCommitted) { auto sessionCheckout = checkOutSession(); auto txnParticipant = TransactionParticipant::get(opCtx()); + txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); unsigned long long beforePreparedThenCommittedCount = ServerTransactionsMetrics::get(opCtx())->getTotalPreparedThenCommitted(); - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); ASSERT_TRUE(txnParticipant.transactionIsCommitted()); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalPreparedThenCommitted(), @@ -1513,14 +1496,13 @@ TEST_F(TransactionsMetricsTest, IncrementCurrentPreparedWithCommit) { auto sessionCheckout = checkOutSession(); auto txnParticipant = TransactionParticipant::get(opCtx()); + txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getCurrentPrepared(), beforeCurrentPrepared + 1U); - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); ASSERT(txnParticipant.transactionIsCommitted()); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getCurrentPrepared(), beforeCurrentPrepared); } @@ -1698,10 +1680,9 @@ TEST_F(TransactionsMetricsTest, TrackCurrentActiveAndInactivePreparedTransaction // Tests that unstashing a transaction puts it into an active state. auto txnParticipant = TransactionParticipant::get(opCtx()); + txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getCurrentActive(), beforeActivePreparedCounter + 1U); @@ -1724,7 +1705,8 @@ TEST_F(TransactionsMetricsTest, TrackCurrentActiveAndInactivePreparedTransaction beforeInactivePreparedCounter); // Tests that committing decrements the active counter only. - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); + ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getCurrentActive(), beforeActivePreparedCounter); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getCurrentInactive(), @@ -1841,12 +1823,10 @@ TEST_F(TransactionsMetricsTest, SingleTransactionStatsPreparedDurationShouldBeSe // Prepare the transaction and extend the duration in the prepared state. const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); tickSource->advance(Microseconds(100)); - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); ASSERT_EQ(txnParticipant.getSingleTransactionStatsForTest().getPreparedDuration( tickSource, tickSource->getTicks()), Microseconds(100)); @@ -1931,9 +1911,6 @@ TEST_F(TransactionsMetricsTest, // Prepare the transaction and extend the duration in the prepared state. const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); - tickSource->advance(Microseconds(100)); // The prepared transaction's duration should have increased. @@ -1944,7 +1921,7 @@ TEST_F(TransactionsMetricsTest, tickSource->advance(Microseconds(100)); // Commit the prepared transaction and check the prepared duration. - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); ASSERT_EQ(txnParticipant.getSingleTransactionStatsForTest().getPreparedDuration( tickSource, tickSource->getTicks()), Microseconds(200)); @@ -2457,9 +2434,6 @@ TEST_F(TransactionsMetricsTest, ReportStashedResources) { // Prepare the transaction and extend the duration in the prepared state. const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); - const long preparedDuration = 10; tickSource->advance(Microseconds(preparedDuration)); @@ -2516,7 +2490,7 @@ TEST_F(TransactionsMetricsTest, ReportStashedResources) { ASSERT(txnParticipant.reportStashedState(opCtx()).isEmpty()); // Commit the transaction. This allows us to release locks. - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); } TEST_F(TransactionsMetricsTest, ReportUnstashedResources) { @@ -2851,7 +2825,7 @@ std::string buildTransactionInfoString( // timeInactiveMicros:2 numYields:0 locks:{ Global: { acquireCount: { r: 6, w: 4 } }, Database: // { acquireCount: { r: 1, w: 1, W: 2 } }, Collection: { acquireCount: { R: 1 } }, oplog: { // acquireCount: { W: 1 } } } wasPrepared:1 totalPreparedDurationMicros:10 - // prepareOpTime:<OpTime> oldestOplogEntryOpTime:<OpTime> finishOpTime:<OpTime> 0ms + // prepareOpTime:<OpTime> oldestOplogEntryOpTime:<OpTime> 0ms StringBuilder expectedTransactionInfo; expectedTransactionInfo << parametersInfo.str() << readTimestampInfo.str() << singleTransactionStatsInfo.str() @@ -2877,10 +2851,6 @@ std::string buildTransactionInfoString( ASSERT(!txnParticipant.getOldestOplogEntryOpTimeForTest()); expectedTransactionInfo << " oldestOplogEntryOpTime:" << oldestOplogEntryOpTime->toString(); } - if (txnParticipant.getFinishOpTimeForTest()) { - expectedTransactionInfo << " finishOpTime:" - << txnParticipant.getFinishOpTimeForTest()->toString(); - } expectedTransactionInfo << ", " << duration_cast<Milliseconds>( txnParticipant.getSingleTransactionStatsForTest().getDuration( @@ -2948,12 +2918,10 @@ TEST_F(TransactionsMetricsTest, TestPreparedTransactionInfoForLogAfterCommit) { auto txnParticipant = TransactionParticipant::get(opCtx()); txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); tickSource->advance(Microseconds(10)); - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); const auto lockerInfo = opCtx()->lockState()->getLockerInfo(boost::none); ASSERT(lockerInfo); @@ -3139,11 +3107,9 @@ TEST_F(TransactionsMetricsTest, LogPreparedTransactionInfoAfterSlowCommit) { txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); const auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - const auto commitTimestamp = - Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); startCapturingLogMessages(); - txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, {}); + txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); stopCapturingLogMessages(); const auto lockerInfo = opCtx()->lockState()->getLockerInfo(boost::none); @@ -3460,128 +3426,6 @@ TEST_F(TxnParticipantTest, ReturnNullTimestampIfNoOldestActiveTimestamp) { ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime()); } -TEST_F(TxnParticipantTest, ProperlyMaintainOldestNonMajorityCommittedOpTimeSet) { - auto sessionCheckout = checkOutSession(); - auto txnParticipant = TransactionParticipant::get(opCtx()); - - // Check that there are no Timestamps in the set. - ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 0U); - - txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction"); - auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - // Check that we added a Timestamp to the set. - ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 1U); - - // Check that the oldest prepareTimestamp is equal to first prepareTimestamp because there is - // only one prepared transaction on this Service. - auto prepareOpTime = ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime(); - ASSERT_EQ(prepareOpTime->getTimestamp(), prepareTimestamp); - - // Check that oldestNonMajorityCommittedOpTimes also has this prepareTimestamp and that the - // pair's finishOpTime is Timestamp::max() because this transaction has not been - // committed/aborted. - auto nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - ASSERT_EQ(nonMajorityCommittedOpTime->getTimestamp(), prepareTimestamp); - auto nonMajorityCommittedOpTimeFinishOpTime = - ServerTransactionsMetrics::get(opCtx())->getFinishOpTimeOfOldestNonMajCommitted_forTest(); - ASSERT_EQ(nonMajorityCommittedOpTimeFinishOpTime->getTimestamp(), Timestamp::max()); - - ASSERT_FALSE(txnParticipant.transactionIsAborted()); - // Since this test uses a mock opObserver, we have to manually set the finishTimestamp on the - // txnParticipant. - auto finishOpTime = repl::OpTime({10, 10}, 0); - repl::ReplClientInfo::forClient(opCtx()->getClient()).setLastOp(finishOpTime); - - txnParticipant.abortActiveTransaction(opCtx()); - ASSERT(txnParticipant.transactionIsAborted()); - - // Make sure that we moved the OpTime from the oldestActiveOplogEntryOpTimes to - // oldestNonMajorityCommittedOpTimes along with the abort/commit oplog entry OpTime - // associated with the transaction. - ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 0U); - ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime()); - - nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - nonMajorityCommittedOpTimeFinishOpTime = - ServerTransactionsMetrics::get(opCtx())->getFinishOpTimeOfOldestNonMajCommitted_forTest(); - ASSERT_FALSE(nonMajorityCommittedOpTime == boost::none); - ASSERT_FALSE(nonMajorityCommittedOpTimeFinishOpTime == boost::none); - ASSERT_EQ(nonMajorityCommittedOpTime->getTimestamp(), prepareTimestamp); - ASSERT(nonMajorityCommittedOpTimeFinishOpTime); - ASSERT_EQ(*nonMajorityCommittedOpTimeFinishOpTime, finishOpTime); - - // If we pass in a mock commit point that is greater than the finish timestamp of the - // oldestNonMajorityCommittedOpTime, it should be removed from the set. This would mean that - // the abort/commit oplog entry is majority committed. - ServerTransactionsMetrics::get(opCtx())->removeOpTimesLessThanOrEqToCommittedOpTime( - repl::OpTime::max()); - nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - ASSERT_FALSE(nonMajorityCommittedOpTime); -} - -TEST_F(TxnParticipantTest, GetOldestNonMajorityCommittedOpTimeReturnsOldestEntry) { - const auto earlierOpTime = repl::OpTime({1, 1}, 0); - const auto earlierFinishOpTime = repl::OpTime({3, 2}, 0); - - const auto middleOpTime = repl::OpTime({1, 2}, 0); - const auto middleFinishOpTime = repl::OpTime({3, 3}, 0); - - const auto laterOpTime = repl::OpTime({1, 3}, 0); - const auto laterFinishOpTime = repl::OpTime({3, 4}, 0); - - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(earlierOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(earlierOpTime, earlierFinishOpTime); - - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(middleOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(middleOpTime, middleFinishOpTime); - - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(laterOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(laterOpTime, laterFinishOpTime); - - auto nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - - ASSERT_EQ(*nonMajorityCommittedOpTime, repl::OpTime({1, 1}, 0)); - - // If we pass in a mock commit point that is greater than the finish timestamp of the - // oldestNonMajorityCommittedOpTime, it should be removed from the set. This would mean that - // the abort/commit oplog entry is majority committed. - ServerTransactionsMetrics::get(opCtx())->removeOpTimesLessThanOrEqToCommittedOpTime( - repl::OpTime::max()); - nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - ASSERT_FALSE(nonMajorityCommittedOpTime); - - // Test that we can remove only a part of the set by passing in a commit point that is only - // greater than or equal to two of the optimes. - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(earlierOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(earlierOpTime, earlierFinishOpTime); - - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(middleOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(middleOpTime, middleFinishOpTime); - - ServerTransactionsMetrics::get(opCtx())->addActiveOpTime(laterOpTime); - ServerTransactionsMetrics::get(opCtx())->removeActiveOpTime(laterOpTime, laterFinishOpTime); - - nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - - ASSERT_EQ(*nonMajorityCommittedOpTime, earlierOpTime); - - ServerTransactionsMetrics::get(opCtx())->removeOpTimesLessThanOrEqToCommittedOpTime( - repl::OpTime({3, 3}, 0)); - nonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - - // earlierOpTime and middleOpTime must have been removed because their finishOpTime are less - // than or equal to the mock commit point. - ASSERT(nonMajorityCommittedOpTime); - ASSERT_EQ(*nonMajorityCommittedOpTime, laterOpTime); -} - TEST_F(TxnParticipantTest, RollbackResetsInMemoryStateOfPreparedTransaction) { auto sessionCheckout = checkOutSession(); @@ -3598,7 +3442,6 @@ TEST_F(TxnParticipantTest, RollbackResetsInMemoryStateOfPreparedTransaction) { // Check that our metrics are initialized to their default values. ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime()); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 0U); - ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime()); // Perform an insert as a part of a transaction so that we have a transaction operation. txnParticipant.unstashTransactionResources(opCtx(), "insert"); @@ -3612,13 +3455,10 @@ TEST_F(TxnParticipantTest, RollbackResetsInMemoryStateOfPreparedTransaction) { // Check that we added a Timestamp to oldestActiveOplogEntryOpTimes. ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 1U); - // Check that the oldest active timestamp and the oldest non majority committed timestamp are - // equal to the prepareTimestamp because there is only one prepared transaction. - auto oldestActiveOpTime = ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime(); - auto oldestNonMajorityCommittedOpTime = - ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime(); - ASSERT_EQ(oldestActiveOpTime->getTimestamp(), prepareTimestamp); - ASSERT_EQ(oldestNonMajorityCommittedOpTime->getTimestamp(), prepareTimestamp); + // Check that the oldest active timestamp is equal to the prepareTimestamp because there is + // only one prepared transaction. + auto prepareOpTime = ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime(); + ASSERT_EQ(prepareOpTime->getTimestamp(), prepareTimestamp); ASSERT_FALSE(txnParticipant.transactionIsAborted()); // Make sure the state of txnParticipant is populated correctly after a prepared transaction. @@ -3639,7 +3479,6 @@ TEST_F(TxnParticipantTest, RollbackResetsInMemoryStateOfPreparedTransaction) { // After calling clearOpTimes, we should no longer have an oldestActiveOpTime. ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestActiveOpTime()); - ASSERT_FALSE(ServerTransactionsMetrics::get(opCtx())->getOldestNonMajorityCommittedOpTime()); ASSERT_EQ(ServerTransactionsMetrics::get(opCtx())->getTotalActiveOpTimes(), 0U); } @@ -3683,7 +3522,6 @@ TEST_F(TxnParticipantTest, CommitPreparedTransactionAsSecondarySetsTheFinishOpTi Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); const auto commitOplogEntryOpTime = repl::OpTime({10, 10}, 0); txnParticipant.commitPreparedTransaction(opCtx(), commitTimestamp, commitOplogEntryOpTime); - ASSERT_EQ(txnParticipant.getFinishOpTimeForTest().get(), commitOplogEntryOpTime); ASSERT_TRUE(txnParticipant.transactionIsCommitted()); } |