From 75f04844c16c161ff963acfa91112009de455a9b Mon Sep 17 00:00:00 2001 From: Maria van Keulen Date: Wed, 18 Mar 2020 16:49:56 -0400 Subject: SERVER-46249 Check individual txn statements for readConcern support --- ...causally_consistent_jscore_txns_passthrough.yml | 1 + .../sharded_jscore_txns_sharded_collections.yml | 1 + .../concurrency/fsm_workloads/CRUD_and_commands.js | 76 +------------- .../CRUD_and_commands_with_createindexes.js | 57 +++++++++++ jstests/core/txns/commands_in_txns_read_concern.js | 110 +++++++++++++++++++++ jstests/core/txns/create_collection.js | 2 +- jstests/core/txns/create_collection_parallel.js | 28 +++++- jstests/core/txns/create_existing_indexes.js | 2 +- jstests/core/txns/create_indexes.js | 2 +- jstests/core/txns/create_indexes_parallel.js | 34 ++++++- .../txns/implicit_collection_creation_in_txn.js | 5 +- ...indexes_in_txn_errors_if_already_in_progress.js | 2 +- src/mongo/db/commands.h | 4 - src/mongo/db/service_entry_point_common.cpp | 33 ++++++- src/mongo/s/commands/strategy.cpp | 5 +- 15 files changed, 264 insertions(+), 98 deletions(-) create mode 100644 jstests/concurrency/fsm_workloads/CRUD_and_commands_with_createindexes.js create mode 100644 jstests/core/txns/commands_in_txns_read_concern.js diff --git a/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml index 3549bc5f8bc..b0ca2824900 100644 --- a/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml @@ -36,6 +36,7 @@ selector: - jstests/core/txns/create_collection_parallel.js - jstests/core/txns/create_indexes.js - jstests/core/txns/create_indexes_parallel.js + - jstests/core/txns/commands_in_txns_read_concern.js # TODO SERVER-46187: maxTimeMS doesn't work correctly for insert on mongos. - jstests/core/txns/write_conflicts_with_non_txns.js diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml index 2510ccda8b8..b0cba47e0d8 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml @@ -32,6 +32,7 @@ selector: - jstests/core/txns/create_collection_parallel.js - jstests/core/txns/create_indexes.js - jstests/core/txns/create_indexes_parallel.js + - jstests/core/txns/commands_in_txns_read_concern.js # TODO SERVER-46187: maxTimeMS doesn't work correctly for insert on mongos. - jstests/core/txns/write_conflicts_with_non_txns.js diff --git a/jstests/concurrency/fsm_workloads/CRUD_and_commands.js b/jstests/concurrency/fsm_workloads/CRUD_and_commands.js index 5e7b24c9c0a..e014ae4990a 100644 --- a/jstests/concurrency/fsm_workloads/CRUD_and_commands.js +++ b/jstests/concurrency/fsm_workloads/CRUD_and_commands.js @@ -1,11 +1,9 @@ 'use strict'; /** - * Perform CRUD operations, some of which may implicitly create collections. Also perform index - * creations which may implicitly create collections. Performs these in parallel with collection- - * dropping operations. + * Perform CRUD operations, some of which may implicitly create collections, in parallel with + * collection-dropping operations. */ - var $config = (function() { const data = {numIds: 10}; @@ -124,28 +122,6 @@ var $config = (function() { } }, - createIndex: function createIndex(db, collName) { - db[collName].createIndex({value: 1}); - }, - - createIdIndex: function createIdIndex(db, collName) { - try { - assertWhenOwnColl.commandWorked(db[collName].createIndex({_id: 1})); - } catch (e) { - if (e.code == ErrorCodes.ConflictingOperationInProgress) { - // createIndex concurrently with dropCollection can throw. - if (TestData.runInsideTransaction) { - e["errorLabels"] = ["TransientTransactionError"]; - throw e; - } - } - } - }, - - dropIndex: function dropIndex(db, collName) { - db[collName].dropIndex({value: 1}); - }, - dropCollection: function dropCollection(db, collName) { db[collName].drop(); } @@ -157,9 +133,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.10, }, insertDocs: { @@ -167,9 +140,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.30, }, updateDocs: { @@ -177,9 +147,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.30, }, readDocs: { @@ -187,9 +154,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.30, }, deleteDocs: { @@ -197,39 +161,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, - dropCollection: 0.30, - }, - createIndex: { - insertDocs: 0.10, - updateDocs: 0.10, - readDocs: 0.10, - deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, - dropCollection: 0.30, - }, - createIdIndex: { - insertDocs: 0.10, - updateDocs: 0.10, - readDocs: 0.10, - deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, - dropCollection: 0.30, - }, - dropIndex: { - insertDocs: 0.10, - updateDocs: 0.10, - readDocs: 0.10, - deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.30, }, dropCollection: { @@ -237,9 +168,6 @@ var $config = (function() { updateDocs: 0.10, readDocs: 0.10, deleteDocs: 0.10, - createIndex: 0.10, - createIdIndex: 0.10, - dropIndex: 0.10, dropCollection: 0.10, } }; diff --git a/jstests/concurrency/fsm_workloads/CRUD_and_commands_with_createindexes.js b/jstests/concurrency/fsm_workloads/CRUD_and_commands_with_createindexes.js new file mode 100644 index 00000000000..dc885e0b575 --- /dev/null +++ b/jstests/concurrency/fsm_workloads/CRUD_and_commands_with_createindexes.js @@ -0,0 +1,57 @@ +'use strict'; + +/** + * Perform CRUD operations, some of which may implicitly create collections. Also perform index + * creations which may implicitly create collections. Performs these in parallel with collection- + * dropping operations. + */ +load('jstests/concurrency/fsm_libs/extend_workload.js'); // for extendWorkload +load('jstests/concurrency/fsm_workloads/CRUD_and_commands.js'); // for $config + +// TODO(SERVER-46971) combine with CRUD_and_commands.js and remove `local` readConcern. +TestData.defaultTransactionReadConcernLevel = "local"; + +var $config = extendWorkload($config, function($config, $super) { + const origStates = Object.keys($config.states); + $config.states = Object.extend({ + createIndex: function createIndex(db, collName) { + db[collName].createIndex({value: 1}); + }, + createIdIndex: function createIdIndex(db, collName) { + try { + assertWhenOwnColl.commandWorked(db[collName].createIndex({_id: 1})); + } catch (e) { + if (e.code == ErrorCodes.ConflictingOperationInProgress) { + // createIndex concurrently with dropCollection can throw. + if (TestData.runInsideTransaction) { + e["errorLabels"] = ["TransientTransactionError"]; + throw e; + } + } + } + }, + + dropIndex: function dropIndex(db, collName) { + db[collName].dropIndex({value: 1}); + } + }, + $super.states); + + let newTransitions = Object.extend({}, $super.transitions); + let exampleState = {}; + origStates.forEach(function(state) { + newTransitions[state]["createIndex"] = 0.10; + newTransitions[state]["createIdIndex"] = 0.10; + newTransitions[state]["dropIndex"] = 0.10; + if (state !== "init" && state !== "dropCollection") { + exampleState = $config.transitions[state]; + } + }); + + newTransitions["createIndex"] = exampleState; + newTransitions["createIdIndex"] = exampleState; + newTransitions["dropIndex"] = exampleState; + + $config.transitions = newTransitions; + return $config; +}); diff --git a/jstests/core/txns/commands_in_txns_read_concern.js b/jstests/core/txns/commands_in_txns_read_concern.js new file mode 100644 index 00000000000..6eae3de2e9b --- /dev/null +++ b/jstests/core/txns/commands_in_txns_read_concern.js @@ -0,0 +1,110 @@ +/* Ensures createCollection and createIndexes are not permitted to run with a readConcern other than + * `local` inside transactions. + * + * @tags: [uses_transactions, + * # Creating collections inside multi-document transactions is supported only in v4.4 + * # onwards. + * requires_fcv_44, + * uses_snapshot_read_concern] + */ +(function() { +"use strict"; + +load("jstests/libs/create_collection_txn_helpers.js"); +load("jstests/libs/create_index_txn_helpers.js"); + +const session = db.getMongo().startSession(); +const collName = jsTestName(); + +let sessionDB = session.getDatabase("test"); +let sessionColl = sessionDB[collName]; +let otherCollName = jsTestName() + "_other"; +let otherColl = sessionDB[otherCollName]; +sessionColl.drop({writeConcern: {w: "majority"}}); +otherColl.drop({writeConcern: {w: "majority"}}); + +jsTest.log("Testing createCollection in a transaction with local readConcern"); +session.startTransaction({readConcern: {level: "local"}, writeConcern: {w: "majority"}}); +createCollAndCRUDInTxn(sessionDB, collName, true /*explicitCreate*/, false /*upsert*/); +assert.commandWorked(session.commitTransaction_forTesting()); +assert.eq(sessionColl.find({}).itcount(), 1); + +sessionColl.drop({writeConcern: {w: "majority"}}); + +jsTest.log("Testing createIndexes in a transaction with local readConcern"); +session.startTransaction({readConcern: {level: "local"}, writeConcern: {w: "majority"}}); +createIndexAndCRUDInTxn(sessionDB, collName, false /*explicitCollCreate*/, false /*multikeyIndex*/); +assert.commandWorked(session.commitTransaction_forTesting()); +assert.eq(sessionColl.find({}).itcount(), 1); +assert.eq(sessionColl.getIndexes().length, 2); + +sessionColl.drop({writeConcern: {w: "majority"}}); +otherColl.drop({writeConcern: {w: "majority"}}); +assert.commandWorked(otherColl.insert({a: 1}, {writeConcern: {w: "majority"}})); +assert.eq(otherColl.find({}).itcount(), 1); + +jsTest.log("Testing createCollection in a transaction with local readConcern, with other " + + "operations preceeding it"); +session.startTransaction({readConcern: {level: "local"}, writeConcern: {w: "majority"}}); +assert.eq(otherColl.find({a: 1}).itcount(), 1); +createCollAndCRUDInTxn(sessionDB, collName, true /*explicitCreate*/, false /*upsert*/); +assert.commandWorked(session.commitTransaction_forTesting()); +assert.eq(sessionColl.find({}).itcount(), 1); + +sessionColl.drop({writeConcern: {w: "majority"}}); +otherColl.drop({writeConcern: {w: "majority"}}); +assert.commandWorked(otherColl.insert({a: 1}, {writeConcern: {w: "majority"}})); +assert.eq(otherColl.find({}).itcount(), 1); + +jsTest.log("Testing createIndexes in a transaction with local readConcern, with other " + + "operations preceeding it"); +session.startTransaction({readConcern: {level: "local"}, writeConcern: {w: "majority"}}); +assert.eq(otherColl.find({a: 1}).itcount(), 1); +createIndexAndCRUDInTxn(sessionDB, collName, false /*explicitCollCreate*/, false /*multikeyIndex*/); +assert.commandWorked(session.commitTransaction_forTesting()); +assert.eq(sessionColl.find({}).itcount(), 1); +assert.eq(sessionColl.getIndexes().length, 2); + +sessionColl.drop({writeConcern: {w: "majority"}}); +otherColl.drop({writeConcern: {w: "majority"}}); +assert.commandWorked(otherColl.insert({a: 1}, {writeConcern: {w: "majority"}})); +assert.eq(otherColl.find({}).itcount(), 1); + +jsTest.log("Testing createCollection in a transaction with non-local readConcern (SHOULD FAIL)"); +session.startTransaction({readConcern: {level: "snapshot"}, writeConcern: {w: "majority"}}); +let res = sessionDB.createCollection(collName); +assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +jsTest.log("Testing createIndexes in a transaction with non-local readConcern (SHOULD FAIL)"); +session.startTransaction({readConcern: {level: "snapshot"}, writeConcern: {w: "majority"}}); +res = sessionColl.runCommand({createIndexes: collName, indexes: [indexSpecs]}); +assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +otherColl.drop({writeConcern: {w: "majority"}}); +assert.commandWorked(otherColl.insert({a: 1}, {writeConcern: {w: "majority"}})); +assert.eq(otherColl.find({}).itcount(), 1); + +jsTest.log("Testing createCollection in a transaction with non-local readConcern, with other " + + "operations preceeding it (SHOULD FAIL)"); +session.startTransaction({readConcern: {level: "snapshot"}, writeConcern: {w: "majority"}}); +assert.eq(otherColl.find({a: 1}).itcount(), 1); +res = sessionDB.createCollection(collName); +assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +otherColl.drop({writeConcern: {w: "majority"}}); +assert.commandWorked(otherColl.insert({a: 1}, {writeConcern: {w: "majority"}})); +assert.eq(otherColl.find({}).itcount(), 1); + +jsTest.log("Testing createIndexes in a transaction with non-local readConcern, with other " + + "operations preceeding it (SHOULD FAIL)"); +session.startTransaction({readConcern: {level: "snapshot"}, writeConcern: {w: "majority"}}); +assert.eq(otherColl.find({a: 1}).itcount(), 1); +res = sessionColl.runCommand({createIndexes: collName, indexes: [indexSpecs]}); +assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +session.endSession(); +}()); diff --git a/jstests/core/txns/create_collection.js b/jstests/core/txns/create_collection.js index be96ebdb740..b2a1f99df2e 100644 --- a/jstests/core/txns/create_collection.js +++ b/jstests/core/txns/create_collection.js @@ -12,7 +12,7 @@ load("jstests/libs/create_collection_txn_helpers.js"); function runCollectionCreateTest(explicitCreate, upsert) { - const session = db.getMongo().startSession({causalConsistency: false}); + const session = db.getMongo().startSession(); const collName = "create_new_collection"; const secondCollName = collName + "_second"; diff --git a/jstests/core/txns/create_collection_parallel.js b/jstests/core/txns/create_collection_parallel.js index 5ff28a06001..fb5fea5668b 100644 --- a/jstests/core/txns/create_collection_parallel.js +++ b/jstests/core/txns/create_collection_parallel.js @@ -14,9 +14,8 @@ function runParallelCollectionCreateTest(explicitCreate, upsert) { const dbName = "test"; const collName = "create_new_collection"; const distinctCollName = collName + "_second"; - const session = db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false}); - const secondSession = - db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false}); + const session = db.getMongo().getDB(dbName).getMongo().startSession(); + const secondSession = db.getMongo().getDB(dbName).getMongo().startSession(); let sessionDB = session.getDatabase("test"); let secondSessionDB = secondSession.getDatabase("test"); @@ -42,6 +41,29 @@ function runParallelCollectionCreateTest(explicitCreate, upsert) { ErrorCodes.NoSuchTransaction); sessionColl.drop({writeConcern: {w: "majority"}}); + distinctSessionColl.drop({writeConcern: {w: "majority"}}); + + jsTest.log("Testing duplicate createCollections, where failing createCollection performs a " + + "successful operation earlier in the transaction."); + + session.startTransaction({writeConcern: {w: "majority"}}); // txn 1 + secondSession.startTransaction({writeConcern: {w: "majority"}}); // txn 2 + + createCollAndCRUDInTxn(secondSessionDB, distinctCollName, explicitCreate, upsert); + createCollAndCRUDInTxn(sessionDB, collName, explicitCreate, upsert); + jsTest.log("Committing transaction 1"); + session.commitTransaction(); + assert.eq(sessionColl.find({}).itcount(), 1); + + assert.commandFailedWithCode(secondSessionDB.runCommand({create: collName}), + ErrorCodes.NamespaceExists); + + assert.commandFailedWithCode(secondSession.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + + assert.eq(distinctSessionColl.find({}).itcount(), 0); + sessionColl.drop({writeConcern: {w: "majority"}}); + distinctSessionColl.drop({writeConcern: {w: "majority"}}); jsTest.log("Testing duplicate createCollections, one inside and one outside a txn"); session.startTransaction({writeConcern: {w: "majority"}}); diff --git a/jstests/core/txns/create_existing_indexes.js b/jstests/core/txns/create_existing_indexes.js index 5ec5dad43fe..9cda50a6164 100644 --- a/jstests/core/txns/create_existing_indexes.js +++ b/jstests/core/txns/create_existing_indexes.js @@ -9,7 +9,7 @@ (function() { "use strict"; -const session = db.getMongo().startSession({causalConsistency: false}); +const session = db.getMongo().startSession(); const collName = "create_existing_indexes"; let sessionDB = session.getDatabase("test"); diff --git a/jstests/core/txns/create_indexes.js b/jstests/core/txns/create_indexes.js index 5d75ddae85f..e160a51b650 100644 --- a/jstests/core/txns/create_indexes.js +++ b/jstests/core/txns/create_indexes.js @@ -12,7 +12,7 @@ load("jstests/libs/create_index_txn_helpers.js"); let doCreateIndexesTest = function(explicitCollectionCreate, multikeyIndex) { - const session = db.getMongo().startSession({causalConsistency: false}); + const session = db.getMongo().startSession(); const collName = "create_new_indexes"; const secondCollName = collName + "_second"; diff --git a/jstests/core/txns/create_indexes_parallel.js b/jstests/core/txns/create_indexes_parallel.js index 6741fd80632..f2e93bad3d3 100644 --- a/jstests/core/txns/create_indexes_parallel.js +++ b/jstests/core/txns/create_indexes_parallel.js @@ -14,9 +14,8 @@ let doParallelCreateIndexesTest = function(explicitCollectionCreate, multikeyInd const dbName = "test"; const collName = "create_new_collection"; const distinctCollName = collName + "_second"; - const session = db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false}); - const secondSession = - db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false}); + const session = db.getMongo().getDB(dbName).getMongo().startSession(); + const secondSession = db.getMongo().getDB(dbName).getMongo().startSession(); let sessionDB = session.getDatabase("test"); let secondSessionDB = secondSession.getDatabase("test"); @@ -66,6 +65,35 @@ let doParallelCreateIndexesTest = function(explicitCollectionCreate, multikeyInd assert.eq(sessionColl.find({}).itcount(), 1); assert.eq(sessionColl.getIndexes().length, 2); + distinctSessionColl.drop({writeConcern: {w: "majority"}}); + sessionColl.drop({writeConcern: {w: "majority"}}); + + jsTest.log("Testing conflicting sequential createIndexes, where failing createIndexes " + + "performs a successful index creation earlier in the transaction."); + session.startTransaction({writeConcern: {w: "majority"}}); // txn 1 + secondSession.startTransaction({writeConcern: {w: "majority"}}); // txn 2 + + createIndexAndCRUDInTxn(sessionDB, distinctCollName, explicitCollectionCreate, multikeyIndex); + createIndexAndCRUDInTxn(secondSessionDB, collName, explicitCollectionCreate, multikeyIndex); + jsTest.log("Committing transaction 2"); + secondSession.commitTransaction(); + assert.eq(secondSessionColl.find({}).itcount(), 1); + assert.eq(secondSessionColl.getIndexes().length, 2); + + // createIndexes takes minimum visible snapshots of new collections into consideration when + // checking for existing indexes. + assert.commandFailedWithCode( + sessionColl.runCommand({createIndexes: collName, indexes: [conflictingIndexSpecs]}), + ErrorCodes.SnapshotUnavailable); + assert.commandFailedWithCode(session.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + + assert.eq(sessionColl.find({}).itcount(), 1); + assert.eq(sessionColl.getIndexes().length, 2); + assert.eq(distinctSessionColl.find({}).itcount(), 0); + assert.eq(distinctSessionColl.getIndexes().length, 0); + + distinctSessionColl.drop({writeConcern: {w: "majority"}}); sessionColl.drop({writeConcern: {w: "majority"}}); jsTest.log( diff --git a/jstests/core/txns/implicit_collection_creation_in_txn.js b/jstests/core/txns/implicit_collection_creation_in_txn.js index ccd59b9e198..491f1b0ec37 100644 --- a/jstests/core/txns/implicit_collection_creation_in_txn.js +++ b/jstests/core/txns/implicit_collection_creation_in_txn.js @@ -11,10 +11,7 @@ const testColl = testDB[collName]; testDB.runCommand({drop: collName, writeConcern: {w: "majority"}}); -const sessionOptions = { - causalConsistency: false -}; -const session = db.getMongo().startSession(sessionOptions); +const session = db.getMongo().startSession(); const sessionDb = session.getDatabase(dbName); const sessionColl = sessionDb[collName]; diff --git a/jstests/noPassthrough/create_indexes_in_txn_errors_if_already_in_progress.js b/jstests/noPassthrough/create_indexes_in_txn_errors_if_already_in_progress.js index 4bd74fb2af5..3f61700bdd8 100644 --- a/jstests/noPassthrough/create_indexes_in_txn_errors_if_already_in_progress.js +++ b/jstests/noPassthrough/create_indexes_in_txn_errors_if_already_in_progress.js @@ -44,7 +44,7 @@ const runSuccessfulIndexBuild = function(dbName, collName, indexSpec, requestNum }; const runFailedIndexBuildInTxn = function(dbName, collName, indexSpec, requestNumber) { - const session = db.getMongo().startSession({causalConsistency: false}); + const session = db.getMongo().startSession(); const sessionDB = session.getDatabase(dbName); const sessionColl = sessionDB[collName]; diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index a2ea871d67b..37f85f0bd05 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -705,10 +705,6 @@ public: * If a readConcern level argument is sent to a command that returns false the command processor * will reject the command, returning an appropriate error message. * - * This only applies when running outside transactions because all commands that are allowed to - * run in a transaction must support all the read concerns that can be used in a - * transaction. - * * Note that this is never called on mongos. Sharded commands are responsible for forwarding * the option to the shards as needed. We rely on the shards to fail the commands in the * cases where it isn't supported. diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 7681eedaf6e..f4497a8ccdf 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -332,10 +332,8 @@ StatusWith _extractReadConcern(OperationContext* opCtx, } } - // If we are starting a transaction, we only need to check whether the read concern is - // appropriate for running a transaction. There is no need to check whether the specific - // command supports the read concern, because all commands that are allowed to run in a - // transaction must support all applicable read concerns. + // If we are starting a transaction, we need to check whether the read concern is + // appropriate for running a transaction. if (startTransaction) { if (!isReadConcernLevelAllowedInTransaction(readConcernArgs.getLevel())) { return {ErrorCodes::InvalidOptions, @@ -356,7 +354,10 @@ StatusWith _extractReadConcern(OperationContext* opCtx, // it is implicitly "local". There is no need to check whether this is supported, because all // commands either support "local" or upconvert the absent readConcern to a stronger level that // they do support; for instance, $changeStream upconverts to RC level "majority". - if (!startTransaction && readConcernArgs.hasLevel()) { + // + // Individual transaction statements are checked later on, after we've unstashed the + // transaction resources. + if (!opCtx->inMultiDocumentTransaction() && readConcernArgs.hasLevel()) { if (!readConcernSupport.readConcernSupport.isOK()) { return readConcernSupport.readConcernSupport.withContext( str::stream() << "Command " << invocation->definition()->getName() @@ -619,6 +620,28 @@ void invokeWithSessionCheckedOut(OperationContext* opCtx, _abortUnpreparedOrStashPreparedTransaction(opCtx, &txnParticipant); }); + if (!opCtx->getClient()->isInDirectClient()) { + const auto& readConcernArgs = repl::ReadConcernArgs::get(opCtx); + + // For replica sets, we do not receive the readConcernArgs of our parent transaction + // statements until we unstash the transaction resources. The below check is necessary to + // ensure commands, including those occurring after the first statement in their respective + // transactions, are checked for readConcern support. Presently, only `create` and + // `createIndexes` do not support readConcern inside transactions. + // TODO(SERVER-46971): Consider how to extend this check to other commands. + auto cmdName = invocation->definition()->getName(); + auto readConcernSupport = invocation->supportsReadConcern(readConcernArgs.getLevel()); + if (readConcernArgs.hasLevel() && + (cmdName == "create"_sd || cmdName == "createIndexes"_sd)) { + if (!readConcernSupport.readConcernSupport.isOK()) { + uassertStatusOK(readConcernSupport.readConcernSupport.withContext( + str::stream() << "Command " << cmdName + << " does not support this transaction's " + << readConcernArgs.toString())); + } + } + } + try { CommandHelpers::runCommandInvocation(opCtx, request, invocation, replyBuilder); } catch (const ExceptionFor&) { diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 0b8d596ab10..fd795080573 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -602,7 +602,10 @@ void runCommand(OperationContext* opCtx, // then it is implicitly "local". There is no need to check whether this is supported, // because all commands either support "local" or upconvert the absent readConcern to a // stronger level that they do support; e.g. $changeStream upconverts to RC "majority". - if (!startTransaction && readConcernArgs.hasLevel()) { + // + // Individual transaction statements are checked later on, after we've unstashed the + // transaction resources. + if (!TransactionRouter::get(opCtx) && readConcernArgs.hasLevel()) { if (!readConcernSupport.readConcernSupport.isOK()) { auto responseBuilder = replyBuilder->getBodyBuilder(); CommandHelpers::appendCommandStatusNoThrow( -- cgit v1.2.1