diff options
26 files changed, 856 insertions, 241 deletions
diff --git a/jstests/concurrency/fsm_workloads/multi_statement_transaction_simple.js b/jstests/concurrency/fsm_workloads/multi_statement_transaction_simple.js index 18ff70c8fdf..ad3f197f644 100644 --- a/jstests/concurrency/fsm_workloads/multi_statement_transaction_simple.js +++ b/jstests/concurrency/fsm_workloads/multi_statement_transaction_simple.js @@ -15,6 +15,7 @@ var $config = (function() { filter: {}, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false }); assertWhenOwnColl.commandWorked(res); @@ -26,6 +27,7 @@ var $config = (function() { getMore: cursorId, collection: collName, txnNumber: NumberLong(txnNumber), + autocommit: false }); assertWhenOwnColl.commandWorked(res); res.cursor.nextBatch.forEach(function(account) { @@ -34,8 +36,8 @@ var $config = (function() { cursorId = res.cursor.id; } // commitTransaction can only be called on the admin database. - assertWhenOwnColl.commandWorked( - sessionDb.adminCommand({commitTransaction: 1, txnNumber: NumberLong(txnNumber)})); + assertWhenOwnColl.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); return total; } @@ -65,13 +67,15 @@ var $config = (function() { update: collName, updates: [{q: {_id: transferFrom}, u: {$inc: {balance: -transferAmount}}}], readConcern: {level: "snapshot"}, + startTransaction: true, autocommit: false }, { update: collName, - updates: [{q: {_id: transferTo}, u: {$inc: {balance: transferAmount}}}] + updates: [{q: {_id: transferTo}, u: {$inc: {balance: transferAmount}}}], + autocommit: false }, - {commitTransaction: 1} + {commitTransaction: 1, autocommit: false} ]; let hasWriteConflict; diff --git a/jstests/core/txns/find_and_modify_in_transaction.js b/jstests/core/txns/find_and_modify_in_transaction.js index aa18dd525c7..24f17b8aa80 100644 --- a/jstests/core/txns/find_and_modify_in_transaction.js +++ b/jstests/core/txns/find_and_modify_in_transaction.js @@ -28,17 +28,22 @@ remove: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(null, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 0, a: 0}, {_id: 1, a: 1}, {_id: 2, a: 2}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a non-matching find-and-modify with update."); res = assert.commandWorked(sessionDb.runCommand({ @@ -47,17 +52,22 @@ update: {$inc: {a: 100}}, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(null, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 0, a: 0}, {_id: 1, a: 1}, {_id: 2, a: 2}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a matching find-and-modify with remove."); res = assert.commandWorked(sessionDb.runCommand({ @@ -66,17 +76,22 @@ remove: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq({_id: 0, a: 0}, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 1, a: 1}, {_id: 2, a: 2}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a matching find-and-modify with update, requesting the old doc."); res = assert.commandWorked(sessionDb.runCommand({ @@ -85,17 +100,22 @@ update: {$inc: {a: 100}}, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq({_id: 1, a: 1}, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 1, a: 101}, {_id: 2, a: 2}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a matching find-and-modify with update, requesting the new doc."); res = assert.commandWorked(sessionDb.runCommand({ @@ -105,17 +125,22 @@ new: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq({_id: 2, a: 102}, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 1, a: 101}, {_id: 2, a: 102}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a matching find-and-modify with upsert, requesting the new doc."); res = assert.commandWorked(sessionDb.runCommand({ @@ -126,17 +151,22 @@ new: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq({_id: 2, a: 202}, res.value); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 1, a: 101}, {_id: 2, a: 202}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a non-matching find-and-modify with upsert, requesting the old doc."); res = assert.commandWorked(sessionDb.runCommand({ @@ -146,6 +176,7 @@ update: {$inc: {a: 100}}, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(null, res.value); @@ -153,13 +184,18 @@ find: collName, filter: {a: 103}, projection: {_id: 0}, - txnNumber: NumberLong(txnNumber) + txnNumber: NumberLong(txnNumber), + autocommit: false })); assert.docEq(res.cursor.firstBatch, [{a: 103}]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a non-matching find-and-modify with upsert, requesting the new doc."); res = assert.commandWorked(sessionDb.runCommand({ @@ -170,15 +206,20 @@ new: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); const newdoc = res.value; assert.eq(204, newdoc.a); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {a: 204}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {a: 204}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [newdoc]); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); }()); diff --git a/jstests/core/txns/multi_delete_in_transaction.js b/jstests/core/txns/multi_delete_in_transaction.js index 32f9438857a..03d0cff1137 100644 --- a/jstests/core/txns/multi_delete_in_transaction.js +++ b/jstests/core/txns/multi_delete_in_transaction.js @@ -27,12 +27,13 @@ deletes: [{q: {a: 99}, limit: 0}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(0, res.n); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 0, a: 0}, {_id: 1, a: 0}, {_id: 2, a: 1}]); // commitTransaction can only be called on the admin database. @@ -41,7 +42,8 @@ txnNumber: NumberLong(txnNumber++), // TODO(russotto): Majority write concern on commit is to avoid a WriteConflictError // writing to the transaction table. - writeConcern: {w: "majority"} + writeConcern: {w: "majority"}, + autocommit: false })); jsTest.log("Do a single-result multi-delete."); @@ -50,16 +52,21 @@ deletes: [{q: {a: 1}, limit: 0}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(1, res.n); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, [{_id: 0, a: 0}, {_id: 1, a: 0}]); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); jsTest.log("Do a multiple-result multi-delete."); res = assert.commandWorked(sessionDb.runCommand({ @@ -67,16 +74,21 @@ deletes: [{q: {a: 0}, limit: 0}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); assert.eq(2, res.n); - res = assert.commandWorked( - sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + res = assert.commandWorked(sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.docEq(res.cursor.firstBatch, []); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, txnNumber: NumberLong(txnNumber++), writeConcern: {w: "majority"}})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber++), + writeConcern: {w: "majority"}, + autocommit: false + })); // Collection should be empty. assert.eq(0, testColl.find().itcount()); diff --git a/jstests/core/txns/multi_statement_transaction.js b/jstests/core/txns/multi_statement_transaction.js index 6d541dee21d..4859860b5f2 100644 --- a/jstests/core/txns/multi_statement_transaction.js +++ b/jstests/core/txns/multi_statement_transaction.js @@ -28,7 +28,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(stmtId++), - // Only the first write in a transaction has autocommit flag. + startTransaction: true, autocommit: false })); @@ -39,7 +39,8 @@ find: collName, filter: {_id: "insert-1"}, txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false }); assert.commandWorked(res); assert.docEq([{_id: "insert-1"}], res.cursor.firstBatch); @@ -49,7 +50,8 @@ insert: collName, documents: [{_id: "insert-2"}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); // Cannot read with default read concern. @@ -61,6 +63,8 @@ assert.commandWorked(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), + stmtId: NumberInt(stmtId++), + autocommit: false })); // Read with default read concern sees the committed transaction. @@ -82,7 +86,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(stmtId++), - // Only the first write in a transaction has autocommmit flag. + startTransaction: true, autocommit: false })); // Batch update in transaction. @@ -91,7 +95,8 @@ updates: [{q: {_id: "update-1"}, u: {$inc: {a: 1}}}, {q: {_id: "update-2"}, u: {$inc: {a: 1}}}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); // Cannot read with default read concern. assert.eq({_id: "update-1", a: 0}, testColl.findOne({_id: "update-1"})); @@ -101,6 +106,8 @@ assert.commandWorked(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), + stmtId: NumberInt(stmtId++), + autocommit: false })); // Read with default read concern sees the commmitted transaction. assert.eq({_id: "update-1", a: 2}, testColl.findOne({_id: "update-1"})); @@ -117,7 +124,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(stmtId++), - // Only the first write in a transaction has autocommit flag. + startTransaction: true, autocommit: false })); @@ -126,13 +133,15 @@ update: collName, updates: [{q: {_id: "doc-1"}, u: {$inc: {a: 1}}}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); assert.commandWorked(sessionDb.runCommand({ update: collName, updates: [{q: {_id: "doc-2"}, u: {$inc: {a: 1}}}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); // Cannot read with default read concern. assert.eq(null, testColl.findOne({_id: "doc-1"})); @@ -143,7 +152,8 @@ find: collName, filter: {$or: [{_id: "doc-1"}, {_id: "doc-2"}]}, txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false }); assert.commandWorked(res); assert.docEq([{_id: "doc-1", a: 1}, {_id: "doc-2", a: 1}], res.cursor.firstBatch); @@ -152,7 +162,10 @@ assert.commandWorked(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), + stmtId: NumberInt(stmtId++), + autocommit: false })); + // Read with default read concern sees the commmitted transaction. assert.eq({_id: "doc-1", a: 1}, testColl.findOne({_id: "doc-1"})); assert.eq({_id: "doc-2", a: 1}, testColl.findOne({_id: "doc-2"})); @@ -169,7 +182,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(stmtId++), - // Only the first write in a transaction has autocommit flag. + startTransaction: true, autocommit: false })); @@ -178,14 +191,16 @@ delete: collName, deletes: [{q: {_id: "doc-1"}, limit: 1}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); // Batch delete. assert.commandWorked(sessionDb.runCommand({ delete: collName, deletes: [{q: {_id: "doc-2"}, limit: 1}, {q: {_id: "doc-3"}, limit: 1}], txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false })); // Cannot read the new doc and still see the to-be removed docs with default read concern. assert.eq({_id: "doc-1"}, testColl.findOne({_id: "doc-1"})); @@ -197,7 +212,8 @@ find: collName, filter: {$or: [{_id: "doc-1"}, {_id: "doc-2"}, {_id: "doc-3"}]}, txnNumber: NumberLong(txnNumber), - stmtId: NumberInt(stmtId++) + stmtId: NumberInt(stmtId++), + autocommit: false }); assert.commandWorked(res); assert.docEq([], res.cursor.firstBatch); @@ -206,7 +222,10 @@ assert.commandWorked(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), + stmtId: NumberInt(stmtId++), + autocommit: false })); + // Read with default read concern sees the commmitted transaction. assert.eq(null, testColl.findOne({_id: "doc-1"})); assert.eq(null, testColl.findOne({_id: "doc-2"})); diff --git a/jstests/core/txns/multi_statement_transaction_abort.js b/jstests/core/txns/multi_statement_transaction_abort.js index 3cff3cab3f3..c1839a3970c 100644 --- a/jstests/core/txns/multi_statement_transaction_abort.js +++ b/jstests/core/txns/multi_statement_transaction_abort.js @@ -25,13 +25,17 @@ documents: [{_id: "insert-1"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), - // Only the first write in a transaction has autocommit flag. + startTransaction: true, autocommit: false })); // Insert a doc within a transaction. - assert.commandWorked(sessionDb.runCommand( - {insert: collName, documents: [{_id: "insert-2"}], txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.runCommand({ + insert: collName, + documents: [{_id: "insert-2"}], + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // Cannot read with default read concern. assert.eq(null, testColl.findOne({_id: "insert-1"})); @@ -39,8 +43,12 @@ assert.eq(null, testColl.findOne({_id: "insert-2"})); // abortTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {abortTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.adminCommand({ + abortTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // Read with default read concern cannot see the aborted transaction. assert.eq(null, testColl.findOne({_id: "insert-1"})); @@ -55,11 +63,16 @@ documents: [{_id: "insert-1"}, {_id: "insert-2"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // Read with default read concern sees the committed transaction. assert.eq({_id: "insert-1"}, testColl.findOne({_id: "insert-1"})); assert.eq({_id: "insert-2"}, testColl.findOne({_id: "insert-2"})); @@ -67,10 +80,13 @@ jsTest.log("Cannot abort empty transaction because it's not in progress"); txnNumber++; // abortTransaction can only be called on the admin database. - assert.commandFailedWithCode( - sessionDb.adminCommand( - {abortTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)}), - ErrorCodes.NoSuchTransaction); + assert.commandFailedWithCode(sessionDb.adminCommand({ + abortTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + }), + ErrorCodes.NoSuchTransaction); jsTest.log("Abort transaction on duplicated key errors"); testColl.drop(); @@ -82,6 +98,7 @@ documents: [{_id: "insert-2"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // But the second insert throws duplicated index key error. @@ -89,6 +106,7 @@ insert: collName, documents: [{_id: "insert-1", x: 0}], txnNumber: NumberLong(txnNumber), + autocommit: false }), ErrorCodes.DuplicateKey); // The error aborts the transaction. @@ -96,7 +114,8 @@ assert.commandFailedWithCode(sessionDb.adminCommand({ commitTransaction: 1, writeConcern: {w: "majority"}, - txnNumber: NumberLong(txnNumber) + txnNumber: NumberLong(txnNumber), + autocommit: false }), ErrorCodes.TransactionAborted); // Verify the documents are the same. @@ -115,6 +134,7 @@ documents: [{_id: "insert-1", from: 1}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); let txnNumber2 = 0; @@ -124,6 +144,7 @@ documents: [{_id: "insert-2", from: 2}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // Insert a doc from session 2 that conflicts with session 1. @@ -131,17 +152,23 @@ insert: collName, documents: [{_id: "insert-1", from: 2}], txnNumber: NumberLong(txnNumber), + autocommit: false }), ErrorCodes.WriteConflict); // Session 1 isn't affected. // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // Transaction on session 2 is aborted. assert.commandFailedWithCode(sessionDb.adminCommand({ commitTransaction: 1, writeConcern: {w: "majority"}, - txnNumber: NumberLong(txnNumber) + txnNumber: NumberLong(txnNumber), + autocommit: false }), ErrorCodes.NoSuchTransaction); // Verify the documents only reflect the first transaction. @@ -155,6 +182,7 @@ documents: [{_id: "running-txn-1"}, {_id: "running-txn-2"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // A higher txnNumber aborts the old and inserts the same document. @@ -164,11 +192,16 @@ documents: [{_id: "running-txn-2"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // Read with default read concern sees the committed transaction but cannot see the aborted one. assert.eq(null, testColl.findOne({_id: "running-txn-1"})); assert.eq({_id: "running-txn-2"}, testColl.findOne({_id: "running-txn-2"})); @@ -184,6 +217,7 @@ batchSize: 2, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); @@ -197,6 +231,7 @@ find: collName, readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); @@ -204,8 +239,12 @@ assert(newReadResult.hasOwnProperty("cursor"), tojson(newReadResult)); assert.eq(0, newReadResult.cursor.id, tojson(newReadResult)); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand( - {commitTransaction: 1, writeConcern: {w: "majority"}, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + writeConcern: {w: "majority"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + })); // TODO: SERVER-33690 Test the old cursor has been killed when the transaction is aborted. diff --git a/jstests/core/txns/multi_statement_transaction_command_args.js b/jstests/core/txns/multi_statement_transaction_command_args.js new file mode 100644 index 00000000000..81656b8d0c3 --- /dev/null +++ b/jstests/core/txns/multi_statement_transaction_command_args.js @@ -0,0 +1,302 @@ +/** + * Verify that multi-statement transaction command arguments behave correctly. + * + * @tags: [uses_transactions] + */ + +(function() { + "use strict"; + load('jstests/libs/uuid_util.js'); + + const dbName = "test"; + const collName = "multi_statement_transaction_command_args"; + const testDB = db.getSiblingDB(dbName); + const testColl = testDB[collName]; + let txnNumber = 0; + + // Set up the test collection. + testColl.drop(); + assert.commandWorked( + testDB.createCollection(testColl.getName(), {writeConcern: {w: "majority"}})); + + // Initiate the session. + const sessionOptions = {causalConsistency: false}; + let session = db.getMongo().startSession(sessionOptions); + let sessionDb = session.getDatabase(dbName); + + /*********************************************************************************************** + * Verify that the 'startTransaction' argument works correctly. + **********************************************************************************************/ + + jsTestLog("Begin a transaction with startTransaction=true and autocommit=false"); + txnNumber++; + + // Start the transaction. + assert.commandWorked(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false + })); + + // Commit the transaction. + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + })); + + jsTestLog("Try to begin a transaction with startTransaction=true and no 'autocommit' field."); + txnNumber++; + + assert.commandFailedWithCode(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should fail. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + }), + ErrorCodes.NoSuchTransaction); + + jsTestLog("Try to begin a transaction with startTransaction=false and autocommit=false"); + txnNumber++; + + // Try to start the transaction. + assert.commandFailedWithCode(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: false, + autocommit: false + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should fail since it was never started. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + }), + ErrorCodes.NoSuchTransaction); + + jsTestLog( + "Try to begin a transaction with 'startTransaction=false' without an 'autocommit' field."); + txnNumber++; + + assert.commandFailedWithCode(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: false + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should fail. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + }), + ErrorCodes.NoSuchTransaction); + + jsTestLog( + "Try to begin a transaction by omitting 'startTransaction' and setting autocommit=false"); + txnNumber++; + + // Start the transaction with an insert. + assert.commandFailedWithCode(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + }), + ErrorCodes.NoSuchTransaction); + + // Committing the transaction should fail. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + }), + ErrorCodes.NoSuchTransaction); + + jsTestLog("Try to start an already in progress transaction."); + txnNumber++; + + // Start the transaction. + assert.commandWorked(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false + })); + + // Try to start the transaction again. + assert.commandFailedWithCode(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false + }), + ErrorCodes.ConflictingOperationInProgress); + + // Commit the transaction. + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + })); + + /*********************************************************************************************** + * Setting autocommit=true or omitting autocommit on a non-initial transaction operation fails. + **********************************************************************************************/ + + jsTestLog("Run a non-initial transaction operation with autocommit=true"); + txnNumber++; + + // Start the transaction with an insert. + assert.commandWorked(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false + })); + + // Try to execute a transaction operation with autocommit=true. It should fail without affecting + // the transaction. + assert.commandFailedWithCode(sessionDb.runCommand({ + insert: collName, + documents: [{_id: txnNumber + "_1"}], + txnNumber: NumberLong(txnNumber), + autocommit: true + }), + ErrorCodes.InvalidOptions); + + // Try to execute a transaction operation without an autocommit field. It should fail without + // affecting the transaction. + assert.commandFailedWithCode(sessionDb.runCommand({ + insert: collName, + documents: [{_id: txnNumber + "_2"}], + txnNumber: NumberLong(txnNumber), + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should succeed. + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + })); + + /*********************************************************************************************** + * Invalid to include autocommit field on an operation not inside a transaction. + **********************************************************************************************/ + + jsTestLog("Run an operation with autocommit=false outside of a transaction."); + txnNumber++; + + assert.commandWorked( + sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)})); + + assert.commandFailedWithCode( + sessionDb.runCommand( + {find: collName, filter: {}, txnNumber: NumberLong(txnNumber), autocommit: false}), + ErrorCodes.InvalidOptions); + + /*********************************************************************************************** + * The 'autocommit' field must be specified on commit/abort commands. + **********************************************************************************************/ + + jsTestLog("Run a commitTransaction command with valid and invalid 'autocommit' field values."); + txnNumber++; + + // Start the transaction with an insert. + assert.commandWorked(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false + })); + + // Committing the transaction should fail if 'autocommit' is omitted. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + writeConcern: {w: "majority"} + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should fail if autocommit=true. + assert.commandFailedWithCode(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + writeConcern: {w: "majority"}, + autocommit: true + }), + ErrorCodes.InvalidOptions); + + // Committing the transaction should succeed. + assert.commandWorked(sessionDb.adminCommand({ + commitTransaction: 1, + txnNumber: NumberLong(txnNumber), + autocommit: false, + writeConcern: {w: "majority"} + })); + + jsTestLog("Run an abortTransaction command with and without an 'autocommit' field"); + txnNumber++; + + // Start the transaction with an insert. + assert.commandWorked(sessionDb.runCommand({ + find: collName, + filter: {}, + readConcern: {level: "snapshot"}, + txnNumber: NumberLong(txnNumber), + startTransaction: true, + autocommit: false, + })); + + // Aborting the transaction should fail if 'autocommit' is omitted. + assert.commandFailedWithCode( + sessionDb.adminCommand({abortTransaction: 1, txnNumber: NumberLong(txnNumber)}), + ErrorCodes.InvalidOptions); + + // Aborting the transaction should fail if autocommit=true. + assert.commandFailedWithCode( + sessionDb.adminCommand( + {abortTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: true}), + ErrorCodes.InvalidOptions); + + // Aborting the transaction should succeed. + assert.commandWorked(sessionDb.adminCommand( + {abortTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); + +}());
\ No newline at end of file diff --git a/jstests/core/txns/no_implicit_collection_creation_in_txn.js b/jstests/core/txns/no_implicit_collection_creation_in_txn.js index f27fc3ee1ca..4b4fc5bda25 100644 --- a/jstests/core/txns/no_implicit_collection_creation_in_txn.js +++ b/jstests/core/txns/no_implicit_collection_creation_in_txn.js @@ -27,13 +27,12 @@ documents: [{_id: "doc"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.eq({_id: "doc"}, testColl.findOne({_id: "doc"})); // Insert fails when the collection does not exist. @@ -43,15 +42,15 @@ documents: [{_id: "doc"}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false }), ErrorCodes.NamespaceNotFound); // commitTransaction can only be called on the admin database. - assert.commandFailedWithCode(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - }), - ErrorCodes.TransactionAborted); + assert.commandFailedWithCode( + sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false}), + ErrorCodes.TransactionAborted); assert.eq(null, testColl.findOne({_id: "doc"})); jsTest.log("Cannot implicitly create a collection in a transaction using update."); @@ -64,13 +63,12 @@ updates: [{q: {_id: "doc"}, u: {$set: {updated: true}}, upsert: true}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.eq({_id: "doc", updated: true}, testColl.findOne({_id: "doc"})); // Update with upsert=true fails when the collection does not exist. @@ -80,15 +78,15 @@ updates: [{q: {_id: "doc"}, u: {$set: {updated: true}}, upsert: true}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false }), ErrorCodes.NamespaceNotFound); // commitTransaction can only be called on the admin database. - assert.commandFailedWithCode(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - }), - ErrorCodes.TransactionAborted); + assert.commandFailedWithCode( + sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false}), + ErrorCodes.TransactionAborted); assert.eq(null, testColl.findOne({_id: "doc"})); // Update without upsert=true succeeds when the collection does not exist. @@ -97,13 +95,12 @@ updates: [{q: {_id: "doc"}, u: {$set: {updated: true}}}], readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.eq(null, testColl.findOne({_id: "doc"})); jsTest.log("Cannot implicitly create a collection in a transaction using findAndModify."); @@ -118,13 +115,12 @@ upsert: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.eq({_id: "doc", updated: true}, testColl.findOne({_id: "doc"})); // findAndModify with upsert=true fails when the collection does not exist. @@ -136,15 +132,15 @@ upsert: true, readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false }), ErrorCodes.NamespaceNotFound); // commitTransaction can only be called on the admin database. - assert.commandFailedWithCode(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - }), - ErrorCodes.TransactionAborted); + assert.commandFailedWithCode( + sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false}), + ErrorCodes.TransactionAborted); assert.eq(null, testColl.findOne({_id: "doc"})); // findAndModify without upsert=true succeeds when the collection does not exist. @@ -154,13 +150,12 @@ update: {$set: {updated: true}}, readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), + startTransaction: true, autocommit: false })); // commitTransaction can only be called on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); assert.eq(null, testColl.findOne({_id: "doc"})); session.endSession(); diff --git a/jstests/core/txns/start_transaction_with_read.js b/jstests/core/txns/start_transaction_with_read.js index 6138e42322a..8cc0bfed48a 100644 --- a/jstests/core/txns/start_transaction_with_read.js +++ b/jstests/core/txns/start_transaction_with_read.js @@ -29,7 +29,7 @@ batchSize: 10, txnNumber: NumberLong(txnNumber), readConcern: {level: "snapshot"}, - // Only the first operation in a transaction has autocommit flag. + startTransaction: true, autocommit: false })); assert.eq(res.cursor.firstBatch, [initialDoc]); @@ -40,11 +40,16 @@ insert: collName, documents: [{_id: "insert-1"}], txnNumber: NumberLong(txnNumber), + autocommit: false })); // Read in the same transaction returns the doc. - res = sessionDb.runCommand( - {find: collName, filter: {_id: "insert-1"}, txnNumber: NumberLong(txnNumber)}); + res = sessionDb.runCommand({ + find: collName, + filter: {_id: "insert-1"}, + txnNumber: NumberLong(txnNumber), + autocommit: false + }); assert.commandWorked(res); assert.docEq([{_id: "insert-1"}], res.cursor.firstBatch); @@ -53,13 +58,12 @@ insert: collName, documents: [{_id: "insert-2"}], txnNumber: NumberLong(txnNumber), + autocommit: false })); // commitTransaction can only be run on the admin database. - assert.commandWorked(sessionDb.adminCommand({ - commitTransaction: 1, - txnNumber: NumberLong(txnNumber), - })); + assert.commandWorked(sessionDb.adminCommand( + {commitTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); // Read with default read concern sees the committed transaction. assert.eq({_id: "insert-1"}, coll.findOne({_id: "insert-1"})); diff --git a/jstests/core/txns/statement_ids_accepted.js b/jstests/core/txns/statement_ids_accepted.js index 6b9acb6ea3e..d11f7662374 100644 --- a/jstests/core/txns/statement_ids_accepted.js +++ b/jstests/core/txns/statement_ids_accepted.js @@ -25,6 +25,7 @@ abortTransaction: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -36,6 +37,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -47,6 +49,7 @@ commitTransaction: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -65,6 +68,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -124,6 +128,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -133,6 +138,7 @@ batchSize: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(1), + autocommit: false })); jsTestLog("Check that findandmodify accepts a statement ID"); @@ -142,6 +148,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -152,6 +159,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -161,6 +169,7 @@ abortTransaction: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + autocommit: false })); jsTestLog("Check that geoNear accepts a statement ID"); assert.writeOK(testColl.insert({geo: {type: "Point", coordinates: [0, 0]}, a: 0}), @@ -187,6 +196,8 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, + autocommit: false })); jsTestLog("Check that geoSearch accepts a statement ID"); @@ -198,6 +209,8 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, + autocommit: false })); jsTestLog("Check that group accepts a statement ID"); @@ -213,6 +226,8 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, + autocommit: false })); jsTestLog("Check that insert accepts a statement ID"); @@ -222,6 +237,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -236,7 +252,7 @@ }, out: {inline: 1}, txnNumber: NumberLong(txnNumber++), - stmtId: NumberInt(0), + stmtId: NumberInt(0) })); jsTestLog("Check that parallelCollectionScan accepts a statement ID"); @@ -254,6 +270,7 @@ prepareTransaction: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -266,6 +283,7 @@ readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), stmtId: NumberInt(0), + startTransaction: true, autocommit: false })); @@ -276,6 +294,7 @@ abortTransaction: 1, txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(1), + autocommit: false })); session.endSession(); diff --git a/jstests/noPassthrough/prepare_conflict.js b/jstests/noPassthrough/prepare_conflict.js index 002e2fdf66d..6cdfb3caf76 100644 --- a/jstests/noPassthrough/prepare_conflict.js +++ b/jstests/noPassthrough/prepare_conflict.js @@ -37,10 +37,10 @@ command += "assert.commandWorked(sessionDB.runCommand({" + "update: '" + collName + "'," + "updates: [{q: " + JSON.stringify(query) + ", u: " + JSON.stringify(update) + "}]," + "txnNumber: NumberLong(" + txn + ")," + "readConcern: {level: 'snapshot'}," + - "autocommit: false" + "}));"; + "autocommit: false," + "startTransaction: true" + "}));"; // Run prepare, which blocks until the failpoint is unset. command += "assert.commandWorked(sessionDB.adminCommand(" + - "{prepareTransaction: 1, txnNumber: NumberLong(" + txn + ")}))"; + "{prepareTransaction: 1, txnNumber: NumberLong(" + txn + "), autocommit: false}))"; return startParallelShell(command, conn.port); } diff --git a/jstests/noPassthrough/prepare_transaction.js b/jstests/noPassthrough/prepare_transaction.js index 6ce2d844da1..53249d5e14e 100644 --- a/jstests/noPassthrough/prepare_transaction.js +++ b/jstests/noPassthrough/prepare_transaction.js @@ -37,26 +37,27 @@ documents: [doc1], readConcern: {level: "snapshot"}, txnNumber: NumberLong(txnNumber), + startTransaction: true, autocommit: false })); // Should not be visible. assert.eq(null, testColl.findOne(doc1)); // Should be visible in this session. - let res = - sessionDB.runCommand({find: collName, filter: doc1, txnNumber: NumberLong(txnNumber)}); + let res = sessionDB.runCommand( + {find: collName, filter: doc1, txnNumber: NumberLong(txnNumber), autocommit: false}); assert.commandWorked(res); assert.docEq([doc1], res.cursor.firstBatch); // Run prepare on the admin db, which immediately runs abort afterwards. - assert.commandWorked( - sessionDB.adminCommand({prepareTransaction: 1, txnNumber: NumberLong(txnNumber)})); + assert.commandWorked(sessionDB.adminCommand( + {prepareTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); // The insert should be visible in this session, but because the prepare command immediately // aborts afterwards, the transaction is rolled back and the insert is not visible. assert.eq(null, testColl.findOne(doc1)); - txnNumber++; - res = sessionDB.runCommand({find: collName, filter: doc1, txnNumber: NumberLong(txnNumber)}); + + res = sessionDB.runCommand({find: collName, filter: doc1}); assert.commandWorked(res); assert.eq([], res.cursor.firstBatch); @@ -73,6 +74,7 @@ updates: [{q: doc1, u: {$inc: {x: 1}}}], txnNumber: NumberLong(txnNumber), readConcern: {level: "snapshot"}, + startTransaction: true, autocommit: false })); @@ -80,18 +82,18 @@ assert.eq(null, testColl.findOne(doc2)); // Should be visible in this session. - res = sessionDB.runCommand({find: collName, filter: doc2, txnNumber: NumberLong(txnNumber)}); + res = sessionDB.runCommand( + {find: collName, filter: doc2, txnNumber: NumberLong(txnNumber), autocommit: false}); assert.commandWorked(res); assert.docEq([doc2], res.cursor.firstBatch); - // Run prepare on the admin db, which immediately runs abort afterwards. - assert.commandWorked( - sessionDB.adminCommand({prepareTransaction: 1, txnNumber: NumberLong(txnNumber)})); + // Run prepare, which immediately runs abort afterwards. + assert.commandWorked(sessionDB.adminCommand( + {prepareTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); // The update should be visible in this session, but because the prepare command immediately // aborts afterwards, the transaction is rolled back and the update is not visible. - txnNumber++; - res = sessionDB.runCommand({find: collName, filter: doc2, txnNumber: NumberLong(txnNumber)}); + res = sessionDB.runCommand({find: collName, filter: doc2}); assert.commandWorked(res); assert.eq([], res.cursor.firstBatch); @@ -110,6 +112,7 @@ deletes: [{q: doc2, limit: 1}], txnNumber: NumberLong(txnNumber), readConcern: {level: "snapshot"}, + startTransaction: true, autocommit: false })); @@ -117,18 +120,18 @@ assert.eq(doc2, testColl.findOne(doc2)); // Should not be visible in this session. - res = sessionDB.runCommand({find: collName, filter: doc2, txnNumber: NumberLong(txnNumber)}); + res = sessionDB.runCommand( + {find: collName, filter: doc2, txnNumber: NumberLong(txnNumber), autocommit: false}); assert.commandWorked(res); assert.docEq([], res.cursor.firstBatch); - // Run prepare on the admin db. - assert.commandWorked( - sessionDB.adminCommand({prepareTransaction: 1, txnNumber: NumberLong(txnNumber)})); + // Run prepare. + assert.commandWorked(sessionDB.adminCommand( + {prepareTransaction: 1, txnNumber: NumberLong(txnNumber), autocommit: false})); // The delete should be visible in this session, but because the prepare command immediately // aborts afterwards, the transaction is rolled back and the document is still visible. - txnNumber++; - res = sessionDB.runCommand({find: collName, filter: doc2, txnNumber: NumberLong(txnNumber)}); + res = sessionDB.runCommand({find: collName, filter: doc2}); assert.commandWorked(res); assert.eq([doc2], res.cursor.firstBatch); replSet.stopSet(); diff --git a/jstests/replsets/transaction_commit_abort_on_secondaries.js b/jstests/replsets/transaction_commit_abort_on_secondaries.js index cbd6607c247..7b4f7ee4a0b 100644 --- a/jstests/replsets/transaction_commit_abort_on_secondaries.js +++ b/jstests/replsets/transaction_commit_abort_on_secondaries.js @@ -10,7 +10,7 @@ const collName = "transaction_commit_abort_on_secondaries"; const rst = new ReplSetTest({name: collName, nodes: 2}); - let nodes = rst.startSet(); + rst.startSet(); // We want a stable topology, so make the secondary unelectable. let config = rst.getReplSetConfig(); config.members[1].priority = 0; diff --git a/src/mongo/db/command_generic_argument.cpp b/src/mongo/db/command_generic_argument.cpp index ef9530d1cb0..ecd0213865b 100644 --- a/src/mongo/db/command_generic_argument.cpp +++ b/src/mongo/db/command_generic_argument.cpp @@ -50,7 +50,7 @@ struct SpecialArgRecord { // If that changes, it should be added. When you add to this list, consider whether you // should also change the filterCommandRequestForPassthrough() function. // clang-format off -static constexpr std::array<SpecialArgRecord, 23> specials{{ +static constexpr std::array<SpecialArgRecord, 24> specials{{ // /-isGeneric // | /-stripFromRequest // | | /-stripFromReply @@ -73,6 +73,7 @@ static constexpr std::array<SpecialArgRecord, 23> specials{{ {"lsid"_sd, 1, 0, 0}, {"txnNumber"_sd, 1, 0, 0}, {"autocommit"_sd, 1, 1, 0}, + {"startTransaction"_sd, 1, 1, 0}, {"stmtId"_sd, 1, 0, 0}, {"$gleStats"_sd, 0, 0, 1}, {"operationTime"_sd, 0, 0, 1}, diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index cc69458f5ad..3fa2181b569 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -115,6 +115,38 @@ struct CommandHelpers { static BSONObj appendMajorityWriteConcern(const BSONObj& cmdObj); /** + * Returns true if the provided argument is one that is handled by the command processing layer + * and should generally be ignored by individual command implementations. In particular, + * commands that fail on unrecognized arguments must not fail for any of these. + */ + static bool isGenericArgument(StringData arg) { + // Not including "help" since we don't pass help requests through to the command parser. + // If that changes, it should be added. When you add to this list, consider whether you + // should also change the filterCommandRequestForPassthrough() function. + return arg == "$audit" || // + arg == "$client" || // + arg == "$configServerState" || // + arg == "$db" || // + arg == "allowImplicitCollectionCreation" || // + arg == "$oplogQueryData" || // + arg == "$queryOptions" || // + arg == "$readPreference" || // + arg == "$replData" || // + arg == "$clusterTime" || // + arg == "maxTimeMS" || // + arg == "readConcern" || // + arg == "databaseVersion" || // + arg == "shardVersion" || // + arg == "tracking_info" || // + arg == "writeConcern" || // + arg == "lsid" || // + arg == "txnNumber" || // + arg == "autocommit" || // + arg == "startTransaction" || // + false; // These comments tell clang-format to keep this line-oriented. + } + + /** * Rewrites cmdObj into a format safe to blindly forward to shards. * * This performs 2 transformations: diff --git a/src/mongo/db/logical_session_id.idl b/src/mongo/db/logical_session_id.idl index d5639ddea69..6c83839e471 100644 --- a/src/mongo/db/logical_session_id.idl +++ b/src/mongo/db/logical_session_id.idl @@ -115,6 +115,11 @@ structs: autocommit: type: bool optional: true + startTransaction: + description: "Used to indicate that a command is the start of a multi-statement + transaction." + type: bool + optional: true SessionsCollectionFetchResultIndividualResult: description: "Individual result" diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp index 625e12dfc91..31a6b9e2bc7 100644 --- a/src/mongo/db/op_observer_impl_test.cpp +++ b/src/mongo/db/op_observer_impl_test.cpp @@ -296,7 +296,7 @@ public: NamespaceString nss, TxnNumber txnNum, StmtId stmtId) { - session->beginOrContinueTxn(opCtx, txnNum, boost::none); + session->beginOrContinueTxn(opCtx, txnNum, boost::none, boost::none); { AutoGetCollection autoColl(opCtx, nss, MODE_IX); diff --git a/src/mongo/db/repl/do_txn_test.cpp b/src/mongo/db/repl/do_txn_test.cpp index ce86cb12eda..67242427e4f 100644 --- a/src/mongo/db/repl/do_txn_test.cpp +++ b/src/mongo/db/repl/do_txn_test.cpp @@ -145,7 +145,10 @@ void DoTxnTest::setUp() { // Set up the transaction and session. _opCtx->setLogicalSessionId(makeLogicalSessionIdForTest()); _opCtx->setTxnNumber(0); // TxnNumber can always be 0 because we have a new session. - _ocs.emplace(_opCtx.get(), true /* checkOutSession */, false /* autocommit */); + _ocs.emplace(_opCtx.get(), + true /* checkOutSession */, + false /* autocommit */, + true /* startTransaction */); OperationContextSession::get(opCtx())->unstashTransactionResources(opCtx()); } diff --git a/src/mongo/db/s/session_catalog_migration_destination_test.cpp b/src/mongo/db/s/session_catalog_migration_destination_test.cpp index b37e7056c53..fdbe9167c58 100644 --- a/src/mongo/db/s/session_catalog_migration_destination_test.cpp +++ b/src/mongo/db/s/session_catalog_migration_destination_test.cpp @@ -242,7 +242,8 @@ public: // requests with txnNumbers aren't allowed. To get around this, we have to manually set // up the session state and perform the insert. initializeOperationSessionInfo(innerOpCtx.get(), insertBuilder.obj(), true, true, true); - OperationContextSession sessionTxnState(innerOpCtx.get(), true, boost::none); + OperationContextSession sessionTxnState( + innerOpCtx.get(), true, boost::none, boost::none); const auto reply = performInserts(innerOpCtx.get(), insertRequest); ASSERT(reply.results.size() == 1); diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 992d7a4b9dc..ad2d1faa7f9 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -541,17 +541,26 @@ void execCommandDatabase(OperationContext* opCtx, const bool shouldCheckoutSession = static_cast<bool>(opCtx->getTxnNumber()) && sessionCheckoutWhitelist.find(command->getName()) != sessionCheckoutWhitelist.cend(); + // Parse the arguments specific to multi-statement transactions. + boost::optional<bool> startMultiDocTxn = boost::none; boost::optional<bool> autocommitVal = boost::none; - if (sessionOptions && sessionOptions->getAutocommit()) { - autocommitVal = *sessionOptions->getAutocommit(); - } else if (sessionOptions && command->getName() == "doTxn") { - // Autocommit is overridden specifically for doTxn to get the oplog entry generation - // behavior used for multi-document transactions. - // The doTxn command still logically behaves as a commit. - autocommitVal = false; + if (sessionOptions) { + startMultiDocTxn = sessionOptions->getStartTransaction(); + autocommitVal = sessionOptions->getAutocommit(); + if (command->getName() == "doTxn") { + // Autocommit and 'startMultiDocTxn' are overridden for 'doTxn' to get the oplog + // entry generation behavior used for multi-document transactions. The 'doTxn' + // command still logically behaves as a commit. + autocommitVal = false; + startMultiDocTxn = true; + } } - OperationContextSession sessionTxnState(opCtx, shouldCheckoutSession, autocommitVal); + // This constructor will check out the session and start a transaction, if necessary. It + // handles the appropriate state management for both multi-statement transactions and + // retryable writes. + OperationContextSession sessionTxnState( + opCtx, shouldCheckoutSession, autocommitVal, startMultiDocTxn); const auto dbname = request.getDatabase().toString(); uassert( diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp index 382bdf136fc..0f600d613da 100644 --- a/src/mongo/db/session.cpp +++ b/src/mongo/db/session.cpp @@ -265,7 +265,8 @@ void Session::refreshFromStorageIfNeeded(OperationContext* opCtx) { void Session::beginOrContinueTxn(OperationContext* opCtx, TxnNumber txnNumber, - boost::optional<bool> autocommit) { + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction) { if (opCtx->getClient()->isInDirectClient()) { return; } @@ -273,7 +274,7 @@ void Session::beginOrContinueTxn(OperationContext* opCtx, invariant(!opCtx->lockState()->isLocked()); stdx::lock_guard<stdx::mutex> lg(_mutex); - _beginOrContinueTxn(lg, txnNumber, autocommit); + _beginOrContinueTxn(lg, txnNumber, autocommit, startTransaction); } void Session::beginOrContinueTxnOnMigration(OperationContext* opCtx, TxnNumber txnNumber) { @@ -414,24 +415,85 @@ bool Session::checkStatementExecutedNoOplogEntryFetch(TxnNumber txnNumber, StmtI void Session::_beginOrContinueTxn(WithLock wl, TxnNumber txnNumber, - boost::optional<bool> autocommit) { + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction) { + + // Check whether the session information needs to be refreshed from disk. _checkValid(wl); + + // Check if the given transaction number is valid for this session. The transaction number must + // be >= the active transaction number. _checkTxnValid(wl, txnNumber); + // Reject argument combinations that are never valid. + uassert(ErrorCodes::InvalidOptions, + "Specifying autocommit=true is not allowed.", + autocommit != boost::optional<bool>(true)); + + uassert(ErrorCodes::InvalidOptions, + "Specifying startTransaction=false is not allowed.", + startTransaction != boost::optional<bool>(false)); + + uassert(ErrorCodes::InvalidOptions, + "Must specify autocommit=false on all operations of a multi-statement transaction.", + !(startTransaction == boost::optional<bool>(true) && autocommit == boost::none)); + + // + // Continue an active transaction. + // if (txnNumber == _activeTxnNumber) { - // Continuing an existing transaction. - uassert(ErrorCodes::IllegalOperation, - "Specifying 'autocommit' is only allowed at the beginning of a transaction", - autocommit == boost::none); + // It is never valid to specify 'startTransaction' on an active transaction. + uassert(ErrorCodes::ConflictingOperationInProgress, + str::stream() << "Cannot specify 'startTransaction' on transaction " << txnNumber + << " since it is already in progress.", + startTransaction == boost::none); + + // Continue a retryable write or a snapshot read. + if (_txnState == MultiDocumentTransactionState::kNone || + _txnState == MultiDocumentTransactionState::kInSnapshotRead) { + uassert(ErrorCodes::InvalidOptions, + "Cannot specify 'autocommit' on an operation not inside a multi-statement " + "transaction.", + autocommit == boost::none); + return; + } + + // Continue a multi-statement transaction. In this case, it is required that + // autocommit=false be given as an argument on the request. Retryable writes and snapshot + // reads will have _autocommit=true, so that is why we verify that _autocommit=false here. + if (!_autocommit) { + uassert( + ErrorCodes::InvalidOptions, + "Must specify autocommit=false on all operations of a multi-statement transaction.", + autocommit == boost::optional<bool>(false)); + } return; } - // Start a new transaction with an autocommit field - _setActiveTxn(wl, txnNumber); - _autocommit = (autocommit != boost::none) ? *autocommit : true; // autocommit defaults to true - _txnState = _autocommit ? MultiDocumentTransactionState::kNone - : MultiDocumentTransactionState::kInProgress; + // + // Start a new transaction. + // + // At this point, the given transaction number must be > _activeTxnNumber. Existence of an + // 'autocommit' field means we interpret this operation as part of a multi-document transaction. + invariant(txnNumber > _activeTxnNumber); + if (autocommit) { + invariant(*autocommit == false); + uassert(ErrorCodes::NoSuchTransaction, + str::stream() << "Given transaction number " << txnNumber + << " does not match any in-progress transactions.", + startTransaction != boost::none); + + _setActiveTxn(wl, txnNumber); + _txnState = MultiDocumentTransactionState::kInProgress; + _autocommit = false; + } else { + invariant(startTransaction == boost::none); + _setActiveTxn(wl, txnNumber); + _autocommit = true; + _txnState = MultiDocumentTransactionState::kNone; + } + invariant(_transactionOperations.empty()); } @@ -509,7 +571,6 @@ void Session::stashTransactionResources(OperationContext* opCtx) { // effectively owns the Session. That is, a user might lock the Client to ensure it doesn't go // away, and then lock the Session owned by that client. We rely on the fact that we are not // using the DefaultLockerImpl to avoid deadlock. - invariant(!isMMAPV1()); stdx::lock_guard<Client> lk(*opCtx->getClient()); stdx::unique_lock<stdx::mutex> lg(_mutex); @@ -627,8 +688,8 @@ void Session::abortActiveTransaction(OperationContext* opCtx) { void Session::_abortTransaction(WithLock wl) { // TODO SERVER-33432 Disallow aborting committed transaction after we implement implicit abort. - // A transaction in kCommitting state will either commit or abort for storage-layer reasons; - // it is too late to abort externally. + // A transaction in kCommitting state will either commit or abort for storage-layer reasons; it + // is too late to abort externally. if (_txnState == MultiDocumentTransactionState::kCommitting || _txnState == MultiDocumentTransactionState::kCommitted) { return; @@ -720,9 +781,8 @@ void Session::_commitTransaction(stdx::unique_lock<stdx::mutex> lk, OperationCon invariant(opObserver); opObserver->onTransactionCommit(opCtx); lk.lock(); - // It's possible some other thread aborted the transaction (e.g. through killSession) - // while the opObserver was running. If that happened, the commit should be reported - // as failed. + // It's possible some other thread aborted the transaction (e.g. through killSession) while + // the opObserver was running. If that happened, the commit should be reported as failed. uassert(ErrorCodes::TransactionAborted, str::stream() << "Transaction " << opCtx->getTxnNumber() << " aborted while attempting to commit", @@ -732,8 +792,8 @@ void Session::_commitTransaction(stdx::unique_lock<stdx::mutex> lk, OperationCon _txnState = MultiDocumentTransactionState::kCommitting; bool committed = false; ON_BLOCK_EXIT([this, &committed, opCtx]() { - // If we're still "committing", the recovery unit failed to commit, and the lock - // is not held. We can't safely use _txnState here, as it is protected by the lock. + // If we're still "committing", the recovery unit failed to commit, and the lock is not + // held. We can't safely use _txnState here, as it is protected by the lock. if (!committed) { stdx::lock_guard<stdx::mutex> lk(_mutex); opCtx->setWriteUnitOfWork(nullptr); @@ -853,7 +913,7 @@ void Session::_registerUpdateCacheOnCommit(OperationContext* opCtx, // entry gets invalidated and immediately refreshed while there were no writes for // newTxnNumber yet. In this case _activeTxnNumber will be less than newTxnNumber // and we will fail to update the cache even though the write was successful. - _beginOrContinueTxn(lg, newTxnNumber, boost::none); + _beginOrContinueTxn(lg, newTxnNumber, boost::none, boost::none); } if (newTxnNumber == _activeTxnNumber) { diff --git a/src/mongo/db/session.h b/src/mongo/db/session.h index 53d72d1d746..486ff569053 100644 --- a/src/mongo/db/session.h +++ b/src/mongo/db/session.h @@ -108,26 +108,31 @@ public: void refreshFromStorageIfNeeded(OperationContext* opCtx); /** - * Starts a new transaction on the session, must be called after refreshFromStorageIfNeeded has - * been called. If an attempt is made to start a transaction with number less than the latest - * transaction this session has seen, an exception will be thrown. + * Starts a new transaction on the session, or continues an already active transaction. In this + * context, a "transaction" is a sequence of operations associated with a transaction number. + * This sequence of operations could be a retryable write or multi-statement transaction. Both + * utilize this method. * - * Sets the autocommit parameter for this transaction. If it is boost::none, no autocommit - * parameter was passed into the request. If this is the first statement of a transaction, - * the autocommit parameter will default to true. + * The 'autocommit' argument represents the value of the field given in the original client + * request. If it is boost::none, no autocommit parameter was passed into the request. Every + * operation that is part of a multi statement transaction must specify 'autocommit=false'. + * 'startTransaction' represents the value of the field given in the original client request, + * and indicates whether this operation is the beginning of a multi-statement transaction. * - * Autocommit can only be specified on the first statement of a transaction. If otherwise, - * this function will throw. - * - * Throws if the session has been invalidated or if an attempt is made to start a transaction - * older than the active. + * Throws an exception if: + * - An attempt is made to start a transaction with number less than the latest + * transaction this session has seen. + * - The session has been invalidated. + * - The values of 'autocommit' and/or 'startTransaction' are inconsistent with the current + * state of the transaction. * * In order to avoid the possibility of deadlock, this method must not be called while holding a - * lock. + * lock. This method must also be called after refreshFromStorageIfNeeded has been called. */ void beginOrContinueTxn(OperationContext* opCtx, TxnNumber txnNumber, - boost::optional<bool> autocommit); + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction); /** * Similar to beginOrContinueTxn except it is used specifically for shard migrations and does * not check or modify the autocommit parameter. @@ -313,7 +318,10 @@ public: const repl::OplogEntry& entry); private: - void _beginOrContinueTxn(WithLock, TxnNumber txnNumber, boost::optional<bool> autocommit); + void _beginOrContinueTxn(WithLock, + TxnNumber txnNumber, + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction); void _beginOrContinueTxnOnMigration(WithLock, TxnNumber txnNumber); diff --git a/src/mongo/db/session_catalog.cpp b/src/mongo/db/session_catalog.cpp index 48869e4c338..d70b68da890 100644 --- a/src/mongo/db/session_catalog.cpp +++ b/src/mongo/db/session_catalog.cpp @@ -237,7 +237,8 @@ void SessionCatalog::_releaseSession(const LogicalSessionId& lsid) { OperationContextSession::OperationContextSession(OperationContext* opCtx, bool checkOutSession, - boost::optional<bool> autocommit) + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction) : _opCtx(opCtx) { if (!opCtx->getLogicalSessionId()) { @@ -265,7 +266,8 @@ OperationContextSession::OperationContextSession(OperationContext* opCtx, checkedOutSession->get()->refreshFromStorageIfNeeded(opCtx); if (opCtx->getTxnNumber()) { - checkedOutSession->get()->beginOrContinueTxn(opCtx, *opCtx->getTxnNumber(), autocommit); + checkedOutSession->get()->beginOrContinueTxn( + opCtx, *opCtx->getTxnNumber(), autocommit, startTransaction); } } diff --git a/src/mongo/db/session_catalog.h b/src/mongo/db/session_catalog.h index 657f6baf405..fcc2164ca18 100644 --- a/src/mongo/db/session_catalog.h +++ b/src/mongo/db/session_catalog.h @@ -249,7 +249,8 @@ class OperationContextSession { public: OperationContextSession(OperationContext* opCtx, bool checkOutSession, - boost::optional<bool> autocommit); + boost::optional<bool> autocommit, + boost::optional<bool> startTransaction); ~OperationContextSession(); diff --git a/src/mongo/db/session_catalog_test.cpp b/src/mongo/db/session_catalog_test.cpp index 57506630023..641c0880ca2 100644 --- a/src/mongo/db/session_catalog_test.cpp +++ b/src/mongo/db/session_catalog_test.cpp @@ -89,7 +89,7 @@ TEST_F(SessionCatalogTest, OperationContextCheckedOutSession) { const TxnNumber txnNum = 20; opCtx()->setTxnNumber(txnNum); - OperationContextSession ocs(opCtx(), true, boost::none); + OperationContextSession ocs(opCtx(), true, boost::none, boost::none); auto session = OperationContextSession::get(opCtx()); ASSERT(session); ASSERT_EQ(*opCtx()->getLogicalSessionId(), session->getSessionId()); @@ -98,7 +98,7 @@ TEST_F(SessionCatalogTest, OperationContextCheckedOutSession) { TEST_F(SessionCatalogTest, OperationContextNonCheckedOutSession) { opCtx()->setLogicalSessionId(makeLogicalSessionIdForTest()); - OperationContextSession ocs(opCtx(), false, boost::none); + OperationContextSession ocs(opCtx(), false, boost::none, boost::none); auto session = OperationContextSession::get(opCtx()); ASSERT(!session); @@ -117,7 +117,7 @@ TEST_F(SessionCatalogTest, GetOrCreateSessionAfterCheckOutSession) { opCtx()->setLogicalSessionId(lsid); boost::optional<OperationContextSession> ocs; - ocs.emplace(opCtx(), true, boost::none); + ocs.emplace(opCtx(), true, boost::none, false); stdx::async(stdx::launch::async, [&] { Client::initThreadIfNotAlready(); @@ -146,11 +146,11 @@ TEST_F(SessionCatalogTest, NestedOperationContextSession) { opCtx()->setLogicalSessionId(makeLogicalSessionIdForTest()); { - OperationContextSession outerScopedSession(opCtx(), true, boost::none); + OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none); { DirectClientSetter inDirectClient(opCtx()); - OperationContextSession innerScopedSession(opCtx(), true, boost::none); + OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none); auto session = OperationContextSession::get(opCtx()); ASSERT(session); @@ -173,7 +173,7 @@ TEST_F(SessionCatalogTest, StashInNestedSessionIsANoop) { opCtx()->setTxnNumber(1); { - OperationContextSession outerScopedSession(opCtx(), true, boost::none); + OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none); Locker* originalLocker = opCtx()->lockState(); RecoveryUnit* originalRecoveryUnit = opCtx()->recoveryUnit(); @@ -198,7 +198,7 @@ TEST_F(SessionCatalogTest, StashInNestedSessionIsANoop) { { // Make it look like we're in a DBDirectClient running a nested operation. DirectClientSetter inDirectClient(opCtx()); - OperationContextSession innerScopedSession(opCtx(), true, boost::none); + OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none); // Indicate that there is a stashed cursor. If we were not in a nested session, this // would ensure that stashing is not a noop. @@ -220,7 +220,7 @@ TEST_F(SessionCatalogTest, UnstashInNestedSessionIsANoop) { opCtx()->setTxnNumber(1); { - OperationContextSession outerScopedSession(opCtx(), true, boost::none); + OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none); Locker* originalLocker = opCtx()->lockState(); RecoveryUnit* originalRecoveryUnit = opCtx()->recoveryUnit(); @@ -239,7 +239,7 @@ TEST_F(SessionCatalogTest, UnstashInNestedSessionIsANoop) { { // Make it look like we're in a DBDirectClient running a nested operation. DirectClientSetter inDirectClient(opCtx()); - OperationContextSession innerScopedSession(opCtx(), true, boost::none); + OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none); OperationContextSession::get(opCtx())->unstashTransactionResources(opCtx()); diff --git a/src/mongo/db/session_test.cpp b/src/mongo/db/session_test.cpp index 5b03cec656c..6e00de8159f 100644 --- a/src/mongo/db/session_test.cpp +++ b/src/mongo/db/session_test.cpp @@ -164,7 +164,7 @@ TEST_F(SessionTest, SessionEntryNotWrittenOnBegin) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 20; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); ASSERT_EQ(sessionId, session.getSessionId()); ASSERT(session.getLastWriteOpTime(txnNum).isNull()); @@ -182,7 +182,7 @@ TEST_F(SessionTest, SessionEntryWrittenAtFirstWrite) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 21; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); const auto opTime = [&] { AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); @@ -215,7 +215,7 @@ TEST_F(SessionTest, StartingNewerTransactionUpdatesThePersistedSession) { session.refreshFromStorageIfNeeded(opCtx()); const auto writeTxnRecordFn = [&](TxnNumber txnNum, StmtId stmtId, repl::OpTime prevOpTime) { - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); WriteUnitOfWork wuow(opCtx()); @@ -254,9 +254,9 @@ TEST_F(SessionTest, StartingOldTxnShouldAssert) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 20; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); - ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum - 1, boost::none), + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum - 1, boost::none, boost::none), AssertionException, ErrorCodes::TransactionTooOld); ASSERT(session.getLastWriteOpTime(txnNum).isNull()); @@ -274,7 +274,7 @@ TEST_F(SessionTest, SessionTransactionsCollectionNotDefaultCreated) { ASSERT(client.runCommand(nss.db().toString(), BSON("drop" << nss.coll()), dropResult)); const TxnNumber txnNum = 21; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); WriteUnitOfWork wuow(opCtx()); @@ -289,7 +289,7 @@ TEST_F(SessionTest, CheckStatementExecuted) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); const auto writeTxnRecordFn = [&](StmtId stmtId, repl::OpTime prevOpTime) { AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); @@ -330,7 +330,7 @@ TEST_F(SessionTest, CheckStatementExecutedForOldTransactionThrows) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); ASSERT_THROWS_CODE(session.checkStatementExecuted(opCtx(), txnNum - 1, 0), AssertionException, @@ -353,7 +353,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryForOldTransactionThrows) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); { AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); @@ -380,7 +380,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryForInvalidatedTransactionThrows) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); WriteUnitOfWork wuow(opCtx()); @@ -400,7 +400,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryCommitIgnoresInvalidation) { session.refreshFromStorageIfNeeded(opCtx()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); { AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); @@ -495,7 +495,7 @@ TEST_F(SessionTest, ErrorOnlyWhenStmtIdBeingCheckedIsNotInCache) { Session session(sessionId); session.refreshFromStorageIfNeeded(opCtx()); - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); auto firstOpTime = ([&]() { AutoGetCollection autoColl(opCtx(), kNss, MODE_IX); @@ -581,7 +581,7 @@ TEST_F(SessionTest, StashAndUnstashResources) { Session session(sessionId); session.refreshFromStorageIfNeeded(opCtx()); - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none); repl::ReadConcernArgs readConcernArgs; ASSERT_OK(readConcernArgs.initialize(BSON("find" @@ -622,7 +622,55 @@ TEST_F(SessionTest, StashAndUnstashResources) { session.commitTransaction(opCtx()); } -TEST_F(SessionTest, CheckAutocommitOnlyAllowedAtBeginningOfTxn) { +TEST_F(SessionTest, StartTransactionRequiredToStartTxn) { + const auto sessionId = makeLogicalSessionIdForTest(); + Session session(sessionId); + session.refreshFromStorageIfNeeded(opCtx()); + + // Autocommit should be true by default. + ASSERT(session.getAutocommit()); + + const TxnNumber txnNum = 100; + + // Must specify startTransaction=true and autocommit=false to start a transaction. + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, false), + AssertionException, + ErrorCodes::InvalidOptions); + + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none), + AssertionException, + ErrorCodes::NoSuchTransaction); + + session.beginOrContinueTxn(opCtx(), txnNum, false, true); + + // Autocommit should be set to false and we should be in a mult-doc transaction. + ASSERT_FALSE(session.getAutocommit()); + ASSERT_TRUE(session.inSnapshotReadOrMultiDocumentTransaction()); +} + +TEST_F(SessionTest, CannotSpecifyStartTransactionOnInProgressTxn) { + const auto sessionId = makeLogicalSessionIdForTest(); + Session session(sessionId); + session.refreshFromStorageIfNeeded(opCtx()); + + // Autocommit should be true by default + ASSERT(session.getAutocommit()); + + const TxnNumber txnNum = 100; + // Must specify startTransaction=true and autocommit=false to start a transaction. + session.beginOrContinueTxn(opCtx(), txnNum, false, true); + + // Autocommit should be set to false and we should be in a mult-doc transaction. + ASSERT_FALSE(session.getAutocommit()); + ASSERT_TRUE(session.inSnapshotReadOrMultiDocumentTransaction()); + + // Cannot try to start a transaction that already started. + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, true), + AssertionException, + ErrorCodes::ConflictingOperationInProgress); +} + +TEST_F(SessionTest, AutocommitRequiredOnEveryTxnOp) { const auto sessionId = makeLogicalSessionIdForTest(); Session session(sessionId); session.refreshFromStorageIfNeeded(opCtx()); @@ -631,16 +679,23 @@ TEST_F(SessionTest, CheckAutocommitOnlyAllowedAtBeginningOfTxn) { ASSERT(session.getAutocommit()); const TxnNumber txnNum = 100; - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); // Autocommit should be set to false ASSERT_FALSE(session.getAutocommit()); - // Trying to set autocommit after the first statement of a transaction - // should throw an error. - ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, true), + // Omitting 'autocommit' after the first statement of a transaction should throw an error. + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none), + AssertionException, + ErrorCodes::InvalidOptions); + + // Setting 'autocommit=true' should throw an error. + ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, true, boost::none), AssertionException, - ErrorCodes::IllegalOperation); + ErrorCodes::InvalidOptions); + + // Including autocommit=false should succeed. + session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none); } TEST_F(SessionTest, SameTransactionPreservesStoredStatements) { @@ -651,14 +706,14 @@ TEST_F(SessionTest, SameTransactionPreservesStoredStatements) { const TxnNumber txnNum = 22; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); WriteUnitOfWork wuow(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); session.addTransactionOperation(opCtx(), operation); ASSERT_BSONOBJ_EQ(operation.toBSON(), session.transactionOperationsForTest()[0].toBSON()); // Re-opening the same transaction should have no effect. - session.beginOrContinueTxn(opCtx(), txnNum, boost::none); + session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none); ASSERT_BSONOBJ_EQ(operation.toBSON(), session.transactionOperationsForTest()[0].toBSON()); } @@ -670,7 +725,7 @@ TEST_F(SessionTest, AbortClearsStoredStatements) { const TxnNumber txnNum = 24; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); session.addTransactionOperation(opCtx(), operation); @@ -693,7 +748,7 @@ TEST_F(SessionTest, EmptyTransactionCommit) { const TxnNumber txnNum = 25; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); // The transaction machinery cannot store an empty locker. Lock::GlobalLock lk(opCtx(), MODE_IX, Date_t::now()); @@ -712,7 +767,7 @@ TEST_F(SessionTest, EmptyTransactionAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); // The transaction machinery cannot store an empty locker. { Lock::GlobalLock lk(opCtx(), MODE_IX, Date_t::now()); } @@ -729,7 +784,7 @@ TEST_F(SessionTest, ConcurrencyOfUnstashAndAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); // The transaction may be aborted without checking out the session. session.abortArbitraryTransaction(); @@ -748,7 +803,7 @@ TEST_F(SessionTest, ConcurrencyOfUnstashAndMigration) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); // The transaction machinery cannot store an empty locker. @@ -775,7 +830,7 @@ TEST_F(SessionTest, ConcurrencyOfStashAndAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); @@ -794,7 +849,7 @@ TEST_F(SessionTest, ConcurrencyOfStashAndMigration) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); @@ -818,7 +873,7 @@ TEST_F(SessionTest, ConcurrencyOfAddTransactionOperationAndAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); @@ -840,7 +895,7 @@ TEST_F(SessionTest, ConcurrencyOfAddTransactionOperationAndMigration) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); @@ -865,7 +920,7 @@ TEST_F(SessionTest, ConcurrencyOfEndTransactionAndRetrieveOperationsAndAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); @@ -886,7 +941,7 @@ TEST_F(SessionTest, ConcurrencyOfEndTransactionAndRetrieveOperationsAndMigration const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); @@ -911,7 +966,7 @@ TEST_F(SessionTest, ConcurrencyOfCommitTransactionAndAbort) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); @@ -931,7 +986,7 @@ TEST_F(SessionTest, ConcurrencyOfCommitTransactionAndMigration) { const TxnNumber txnNum = 26; opCtx()->setLogicalSessionId(sessionId); opCtx()->setTxnNumber(txnNum); - session.beginOrContinueTxn(opCtx(), txnNum, false); + session.beginOrContinueTxn(opCtx(), txnNum, false, true); session.unstashTransactionResources(opCtx()); auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0)); diff --git a/src/mongo/shell/session.js b/src/mongo/shell/session.js index 3b65fd32ce5..8914b0680b7 100644 --- a/src/mongo/shell/session.js +++ b/src/mongo/shell/session.js @@ -654,13 +654,13 @@ var { cmdObjUnwrapped.txnNumber = new NumberLong(_txnNumber); } - // readConcern and autocommit can only be specified on the first statement in a - // transaction. + // All operations of a multi-statement transaction must specify autocommit=false. + cmdObjUnwrapped.autocommit = false; + + // 'readConcern' and 'startTransaction' can only be specified on the first statement in + // a transaction. if (_firstStatement) { - // TODO: As a part of SERVER-34052, we might also need to specify - // `cmdObjUnwrapped.startTransaction = 1` on the first statement of a multi - // statement txn. - cmdObjUnwrapped.autocommit = false; + cmdObjUnwrapped.startTransaction = true; if (_txnOptions.getTxnReadConcern() !== undefined) { cmdObjUnwrapped.readConcern = _txnOptions.getTxnReadConcern(); } |