SERVER-41050 Ban txnNumbers outside of transactions and retryable writes

7 files changed, 82 insertions, 111 deletions

diff --git a/jstests/core/txns/commands_banning_txnnumber_outside_transactions.js b/jstests/core/txns/commands_banning_txnnumber_outside_transactions.js
new file mode 100644
index 00000000000..f4a24b62c2e
--- /dev/null
+++ b/jstests/core/txns/commands_banning_txnnumber_outside_transactions.js
@@ -0,0 +1,58 @@
+// Test that commands other than retryable writes may not use txnNumber outside transactions.
+(function() {
+    "use strict";
+
+    const isMongos = assert.commandWorked(db.runCommand("ismaster")).msg === "isdbgrid";
+
+    const session = db.getMongo().startSession();
+    const sessionDb = session.getDatabase("admin");
+
+    const nonRetryableWriteCommands = [
+        // Commands that are allowed in transactions.
+        {aggregate: 1},
+        {commitTransaction: 1},
+        {distinct: "c"},
+        {find: "c"},
+        {getMore: NumberLong(1), collection: "c"},
+        {killCursors: 1},
+        // A selection of commands that are not allowed in transactions.
+        {count: 1},
+        {explain: {find: "c"}},
+        {filemd5: 1},
+        {isMaster: 1},
+        {buildInfo: 1},
+        {ping: 1},
+        {listCommands: 1},
+        {create: "c"},
+        {drop: 1},
+        {createIndexes: 1},
+        {mapReduce: "c"}
+    ];
+
+    const nonRetryableWriteCommandsMongodOnly = [
+        // Commands that are allowed in transactions.
+        {coordinateCommitTransaction: 1, participants: []},
+        {geoSearch: 1},
+        {prepareTransaction: 1},
+        // A selection of commands that are not allowed in transactions.
+        {applyOps: 1}
+    ];
+
+    nonRetryableWriteCommands.forEach(function(command) {
+        jsTest.log("Testing command: " + tojson(command));
+        assert.commandFailedWithCode(
+            sessionDb.runCommand(Object.assign({}, command, {txnNumber: NumberLong(0)})),
+            [50768, 50889]);
+    });
+
+    if (!isMongos) {
+        nonRetryableWriteCommandsMongodOnly.forEach(function(command) {
+            jsTest.log("Testing command: " + tojson(command));
+            assert.commandFailedWithCode(
+                sessionDb.runCommand(Object.assign({}, command, {txnNumber: NumberLong(0)})),
+                [50768, 50889]);
+        });
+    }
+
+    session.endSession();
+}());
diff --git a/jstests/core/txns/commands_not_allowed_in_txn.js b/jstests/core/txns/commands_not_allowed_in_txn.js
index e78ec215b44..7300de24598 100644
--- a/jstests/core/txns/commands_not_allowed_in_txn.js
+++ b/jstests/core/txns/commands_not_allowed_in_txn.js
@@ -97,31 +97,14 @@
     }
 
     //
-    // Test commands that check out the session but are not allowed in multi-document
-    // transactions.
+    // Test a selection of commands that are not allowed in transactions.
     //
 
-    const sessionCommands = [
+    const commands = [
         {count: collName},
         {count: collName, query: {a: 1}},
         {explain: {find: collName}},
         {filemd5: 1, root: "fs"},
-    ];
-
-    // There is no applyOps command on mongos.
-    if (!isMongos) {
-        sessionCommands.push(
-            {applyOps: [{op: "u", ns: testColl.getFullName(), o2: {_id: 0}, o: {$set: {a: 5}}}]});
-    }
-
-    sessionCommands.forEach(testCommand);
-
-    //
-    // Test a selection of commands that do not check out the session. It is illegal to provide a
-    // 'txnNumber' on these commands.
-    //
-
-    const nonSessionCommands = [
         {isMaster: 1},
         {buildInfo: 1},
         {ping: 1},
@@ -139,14 +122,13 @@
         {mapReduce: collName, map: function() {}, reduce: function(key, vals) {}, out: {inline: 1}},
     ];
 
-    nonSessionCommands.forEach(testCommand);
+    // There is no applyOps command on mongos.
+    if (!isMongos) {
+        commands.push(
+            {applyOps: [{op: "u", ns: testColl.getFullName(), o2: {_id: 0}, o: {$set: {a: 5}}}]});
+    }
 
-    nonSessionCommands.forEach(function(command) {
-        setup();
-        assert.commandFailedWithCode(
-            sessionDb.runCommand(Object.assign({}, command, {txnNumber: NumberLong(++txnNumber)})),
-            [50768, 50889]);
-    });
+    commands.forEach(testCommand);
 
     //
     // Test that doTxn is not allowed at positions after the first in transactions.
diff --git a/jstests/core/txns/errors_on_committed_transaction.js b/jstests/core/txns/errors_on_committed_transaction.js
index b27442f05b6..221750c31bd 100644
--- a/jstests/core/txns/errors_on_committed_transaction.js
+++ b/jstests/core/txns/errors_on_committed_transaction.js
@@ -40,8 +40,7 @@
     jsTestLog("Test the error precedence when calling prepare on a committed transaction but not " +
               "providing autocommit to prepareTransaction.");
     assert.commandFailedWithCode(
-        sessionDB.adminCommand({prepareTransaction: 1, txnNumber: txnNumber}),
-        ErrorCodes.IncompleteTransactionHistory);
+        sessionDB.adminCommand({prepareTransaction: 1, txnNumber: txnNumber}), 50768);
 
     jsTestLog("Test the error precedence when calling prepare on a committed transaction and " +
               "providing startTransaction to prepareTransaction.");
diff --git a/jstests/core/txns/multi_statement_transaction_command_args.js b/jstests/core/txns/multi_statement_transaction_command_args.js
index b80735f20b3..f0e4ce29759 100644
--- a/jstests/core/txns/multi_statement_transaction_command_args.js
+++ b/jstests/core/txns/multi_statement_transaction_command_args.js
@@ -236,8 +236,7 @@
     jsTestLog("Run an operation with autocommit=false outside of a transaction.");
     txnNumber++;
 
-    assert.commandWorked(
-        sessionDb.runCommand({find: collName, filter: {}, txnNumber: NumberLong(txnNumber)}));
+    assert.commandWorked(sessionDb.runCommand({find: collName, filter: {}}));
 
     assert.commandFailedWithCode(
         sessionDb.runCommand(
@@ -262,24 +261,12 @@
     }));
 
     // Committing the transaction should fail if 'autocommit' is omitted.
-    {
-        // On mongos the router will fail with InvalidOptions, but on a shard
-        // it will return IncompleteTransactionHistory.
-        const expectedErrorCode = (() => {
-            if (FixtureHelpers.isMongos(sessionDb)) {
-                return ErrorCodes.InvalidOptions;
-            } else {
-                return ErrorCodes.IncompleteTransactionHistory;
-            }
-        })();
-
-        assert.commandFailedWithCode(sessionDb.adminCommand({
-            commitTransaction: 1,
-            txnNumber: NumberLong(txnNumber),
-            writeConcern: {w: "majority"}
-        }),
-                                     expectedErrorCode);
-    }
+    assert.commandFailedWithCode(sessionDb.adminCommand({
+        commitTransaction: 1,
+        txnNumber: NumberLong(txnNumber),
+        writeConcern: {w: "majority"}
+    }),
+                                 50768);
 
     // Committing the transaction should fail if autocommit=true.
     assert.commandFailedWithCode(sessionDb.adminCommand({
@@ -312,21 +299,8 @@
     }));
 
     // Aborting the transaction should fail if 'autocommit' is omitted.
-    {
-        // On mongos the router will fail with InvalidOptions, but on a shard
-        // it will return IncompleteTransactionHistory.
-        const expectedErrorCode = (() => {
-            if (FixtureHelpers.isMongos(sessionDb)) {
-                return ErrorCodes.InvalidOptions;
-            } else {
-                return ErrorCodes.IncompleteTransactionHistory;
-            }
-        })();
-
-        assert.commandFailedWithCode(
-            sessionDb.adminCommand({abortTransaction: 1, txnNumber: NumberLong(txnNumber)}),
-            expectedErrorCode);
-    }
+    assert.commandFailedWithCode(
+        sessionDb.adminCommand({abortTransaction: 1, txnNumber: NumberLong(txnNumber)}), 50768);
 
     // Aborting the transaction should fail if autocommit=true.
     assert.commandFailedWithCode(
diff --git a/jstests/core/txns/no_read_concern_snapshot_outside_txn.js b/jstests/core/txns/no_read_concern_snapshot_outside_txn.js
index 5634d7df473..2b69510ecde 100644
--- a/jstests/core/txns/no_read_concern_snapshot_outside_txn.js
+++ b/jstests/core/txns/no_read_concern_snapshot_outside_txn.js
@@ -22,16 +22,13 @@
     let txnNumber = 0;
     let stmtId = 0;
 
-    function tryCommands({testDB, assignTxnNumber, message}) {
+    function tryCommands({testDB, message}) {
         jsTestLog("Verify that inserts cannot use readConcern snapshot " + message);
         let cmd = {
             insert: collName,
             documents: [{_id: 0}],
             readConcern: {level: "snapshot"},
         };
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
 
         jsTestLog("Verify that updates cannot use readConcern snapshot " + message);
@@ -40,9 +37,6 @@
             updates: [{q: {_id: 0}, u: {$set: {x: 1}}}],
             readConcern: {level: "snapshot"},
         };
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
 
         jsTestLog("Verify that deletes cannot use readConcern snapshot " + message);
@@ -51,9 +45,6 @@
             deletes: [{q: {_id: 0}, limit: 1}],
             readConcern: {level: "snapshot"},
         };
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
 
         jsTestLog("Verify that findAndModify cannot use readConcern snapshot " + message);
@@ -63,32 +54,18 @@
             remove: true,
             readConcern: {level: "snapshot"},
         };
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
 
         jsTestLog("Verify that finds cannot use readConcern snapshot " + message);
         cmd = {find: collName, readConcern: {level: "snapshot"}};
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
 
         jsTestLog("Verify that aggregate cannot use readConcern snapshot " + message);
         cmd = {aggregate: collName, pipeline: [], readConcern: {level: "snapshot"}};
-        if (assignTxnNumber) {
-            Object.assign(cmd, {txnNumber: NumberLong(txnNumber++), stmtId: NumberInt(stmtId)});
-        }
         assert.commandFailedWithCode(testDB.runCommand(cmd), ErrorCodes.InvalidOptions);
     }
-    tryCommands({
-        testDB: sessionDb,
-        assignTxnNumber: true,
-        message: "in session using snapshot read syntax."
-    });
-    tryCommands({testDB: sessionDb, assignTxnNumber: false, message: "in session."});
-    tryCommands({testDB: testDB, assignTxnNumber: false, message: "outside session."});
+    tryCommands({testDB: sessionDb, message: "in session."});
+    tryCommands({testDB: testDB, message: "outside session."});
 
     session.endSession();
 }());
diff --git a/jstests/core/txns/prepare_nonexistent_transaction.js b/jstests/core/txns/prepare_nonexistent_transaction.js
index 5ef5f74fa88..20f2a51a2a7 100644
--- a/jstests/core/txns/prepare_nonexistent_transaction.js
+++ b/jstests/core/txns/prepare_nonexistent_transaction.js
@@ -77,15 +77,13 @@
     assert.commandFailedWithCode(sessionDB.adminCommand({prepareTransaction: 1, autocommit: false}),
                                  ErrorCodes.InvalidOptions);
 
-    // It isn't ideal that NoSuchTransaction is thrown instead of InvalidOptions here, but it's okay
-    // to leave as is for now since this case fails in some way.
     jsTestLog("Test the error precedence when calling prepare on a nonexistent transaction but " +
               "not providing autocommit to prepareTransaction.");
     assert.commandFailedWithCode(sessionDB.adminCommand({
         prepareTransaction: 1,
         txnNumber: NumberLong(session.getTxnNumber_forTesting() + 1),
     }),
-                                 ErrorCodes.NoSuchTransaction);
+                                 50768);
 
     jsTestLog("Test the error precedence when calling prepare on a nonexistent transaction and " +
               "providing startTransaction to prepareTransaction.");
diff --git a/jstests/core/txns/statement_ids_accepted.js b/jstests/core/txns/statement_ids_accepted.js
index 669dfa19fbf..55aa90ce782 100644
--- a/jstests/core/txns/statement_ids_accepted.js
+++ b/jstests/core/txns/statement_ids_accepted.js
@@ -1,5 +1,5 @@
-// Makes sure all commands which are supposed to take statement ids do.  This should test the
-// commands in the sessionCheckOutWhiteList in service_entry_point_common.cpp.
+// Makes sure all commands which are supposed to take statement ids do.  This should test all
+// commands that are allowed in transactions.
 // @tags: [uses_transactions, uses_prepare_transaction]
 (function() {
     "use strict";
@@ -91,23 +91,6 @@
 
     // The doTxn command is intentionally left out.
 
-    jsTestLog("Check that explain accepts a statement ID");
-    assert.commandWorked(sessionDb.runCommand({
-        explain: {
-            delete: collName,
-            deletes: [{q: {}, limit: 1}],
-        },
-        txnNumber: NumberLong(txnNumber++),
-        stmtId: NumberInt(0),
-    }));
-
-    jsTestLog("Check that filemd5 accepts a statement ID");
-    assert.commandWorked(sessionDb.runCommand({
-        filemd5: "nofile",
-        txnNumber: NumberLong(txnNumber++),
-        stmtId: NumberInt(0),
-    }));
-
     jsTestLog("Check that find and getmore accept a statement ID");
     // Put in some data to find so getMore has a cursor to use.
     assert.writeOK(testColl.insert([{_id: 0}, {_id: 1}], {writeConcern: {w: "majority"}}));