From ab533026f3c32a4305a641a9b9a389ce5f769890 Mon Sep 17 00:00:00 2001 From: Maria van Keulen Date: Tue, 28 Apr 2020 17:23:52 -0400 Subject: SERVER-46900 Prohibit transaction operations on system.profile (cherry picked from commit e4821bc15a083d1d156fa1ff7330ad5a1f985f9c) --- .../core/txns/banned_collection_reads_in_txn.js | 45 ++++++++++++++++++++++ .../txns/no_reads_from_system_dot_views_in_txn.js | 40 ------------------- .../txns/no_writes_to_system_collections_in_txn.js | 7 +++- .../network_error_and_txn_override.js | 21 ++++++++-- .../libs/txns/txn_passthrough_runner_selftest.js | 12 ++++-- 5 files changed, 77 insertions(+), 48 deletions(-) create mode 100644 jstests/core/txns/banned_collection_reads_in_txn.js delete mode 100644 jstests/core/txns/no_reads_from_system_dot_views_in_txn.js (limited to 'jstests') diff --git a/jstests/core/txns/banned_collection_reads_in_txn.js b/jstests/core/txns/banned_collection_reads_in_txn.js new file mode 100644 index 00000000000..5c3dbe070b0 --- /dev/null +++ b/jstests/core/txns/banned_collection_reads_in_txn.js @@ -0,0 +1,45 @@ +// Tests that it is illegal to read from system.views and system.profile within a transaction. +// @tags: [requires_fcv_44, uses_transactions, uses_snapshot_read_concern] +(function() { +"use strict"; + +load("jstests/libs/fixture_helpers.js"); // For 'FixtureHelpers'. + +const session = db.getMongo().startSession(); + +// Use a custom database to avoid conflict with other tests. +const testDB = session.getDatabase("no_reads_from_system_colls_in_txn"); +assert.commandWorked(testDB.dropDatabase()); + +testDB.runCommand({create: "foo", viewOn: "bar", pipeline: []}); + +session.startTransaction({readConcern: {level: "snapshot"}}); +assert.commandFailedWithCode(testDB.runCommand({find: "system.views", filter: {}}), 51071); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +session.startTransaction({readConcern: {level: "snapshot"}}); +assert.commandFailedWithCode(testDB.runCommand({find: "system.profile", filter: {}}), + ErrorCodes.OperationNotSupportedInTransaction); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + +if (FixtureHelpers.isMongos(testDB)) { + // The rest of the test is concerned with a find by UUID which is not supported against + // mongos. + return; +} + +const collectionInfos = + new DBCommandCursor(testDB, assert.commandWorked(testDB.runCommand({listCollections: 1}))); +let systemViewsUUID = null; +while (collectionInfos.hasNext()) { + const next = collectionInfos.next(); + if (next.name === "system.views") { + systemViewsUUID = next.info.uuid; + } +} +assert.neq(null, systemViewsUUID, "did not find UUID for system.views"); + +session.startTransaction({readConcern: {level: "snapshot"}}); +assert.commandFailedWithCode(testDB.runCommand({find: systemViewsUUID, filter: {}}), 51070); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); +}()); diff --git a/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js b/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js deleted file mode 100644 index 808bc8dbb72..00000000000 --- a/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js +++ /dev/null @@ -1,40 +0,0 @@ -// Tests that it is illegal to read from system.views within a transaction. -// @tags: [uses_transactions, uses_snapshot_read_concern] -(function() { -"use strict"; - -load("jstests/libs/fixture_helpers.js"); // For 'FixtureHelpers'. - -const session = db.getMongo().startSession({causalConsistency: false}); - -// Use a custom database to avoid conflict with other tests that use system.views. -const testDB = session.getDatabase("no_reads_from_system_dot_views_in_txn"); -assert.commandWorked(testDB.dropDatabase()); - -testDB.runCommand({create: "foo", viewOn: "bar", pipeline: []}); - -session.startTransaction({readConcern: {level: "snapshot"}}); -assert.commandFailedWithCode(testDB.runCommand({find: "system.views", filter: {}}), 51071); -assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); - -if (FixtureHelpers.isMongos(testDB)) { - // The rest of the test is concerned with a find by UUID which is not supported against - // mongos. - return; -} - -const collectionInfos = - new DBCommandCursor(testDB, assert.commandWorked(testDB.runCommand({listCollections: 1}))); -let systemViewsUUID = null; -while (collectionInfos.hasNext()) { - const next = collectionInfos.next(); - if (next.name === "system.views") { - systemViewsUUID = next.info.uuid; - } -} -assert.neq(null, systemViewsUUID, "did not find UUID for system.views"); - -session.startTransaction({readConcern: {level: "snapshot"}}); -assert.commandFailedWithCode(testDB.runCommand({find: systemViewsUUID, filter: {}}), 51070); -assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); -}()); diff --git a/jstests/core/txns/no_writes_to_system_collections_in_txn.js b/jstests/core/txns/no_writes_to_system_collections_in_txn.js index ab842be7cc1..29882a07499 100644 --- a/jstests/core/txns/no_writes_to_system_collections_in_txn.js +++ b/jstests/core/txns/no_writes_to_system_collections_in_txn.js @@ -1,5 +1,5 @@ // Tests that it is illegal to write to system collections within a transaction. -// @tags: [uses_transactions, uses_snapshot_read_concern] +// @tags: [requires_fcv_44, uses_transactions, uses_snapshot_read_concern] (function() { "use strict"; @@ -45,6 +45,11 @@ session.startTransaction({readConcern: {level: "snapshot"}}); assert.commandFailedWithCode(systemColl.insert({name: "new"}), 50791); assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); +session.startTransaction({readConcern: {level: "snapshot"}}); +assert.commandFailedWithCode(testDB.getCollection("system.profile").insert({name: "new"}), + ErrorCodes.OperationNotSupportedInTransaction); +assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction); + session.startTransaction({readConcern: {level: "snapshot"}}); assert.commandFailedWithCode(systemDotViews.insert({_id: "new.view", viewOn: "bar", pipeline: []}), 50791); diff --git a/jstests/libs/override_methods/network_error_and_txn_override.js b/jstests/libs/override_methods/network_error_and_txn_override.js index de7c9d24a92..798d9bf7fda 100644 --- a/jstests/libs/override_methods/network_error_and_txn_override.js +++ b/jstests/libs/override_methods/network_error_and_txn_override.js @@ -587,6 +587,16 @@ function isCommandNonTxnGetMore(cmdName, cmdObj) { return cmdName === "getMore" && nonTxnAggCursorSet[cmdObj.getMore]; } +function isNamespaceSystemDotProfile(cmdObj) { + // No operations on system.profile are permitted inside transactions (see SERVER-46900). + for (let val of Object.values(cmdObj)) { + if (typeof val === 'string' && val.endsWith('system.profile')) { + return true; + } + } + return false; +} + function setupTransactionCommand(conn, dbName, cmdName, cmdObj, lsid) { // We want to overwrite whatever read and write concern is already set. delete cmdObj.readConcern; @@ -596,8 +606,10 @@ function setupTransactionCommand(conn, dbName, cmdName, cmdObj, lsid) { // use transactions. const driverSession = conn.getDB(dbName).getSession(); const commandSupportsTransaction = TransactionsUtil.commandSupportsTxn(dbName, cmdName, cmdObj); - if (commandSupportsTransaction && driverSession.getSessionId() !== null && - !isCommandNonTxnGetMore(cmdName, cmdObj)) { + const isSystemDotProfile = isNamespaceSystemDotProfile(cmdObj); + + if (commandSupportsTransaction && !isSystemDotProfile && + driverSession.getSessionId() !== null && !isCommandNonTxnGetMore(cmdName, cmdObj)) { if (isNested()) { // Nested commands should never start a new transaction. } else if (ops.length === 0) { @@ -616,7 +628,10 @@ function setupTransactionCommand(conn, dbName, cmdName, cmdObj, lsid) { continueTransaction(conn, dbName, cmdName, cmdObj); } else { - if (ops.length > 0 && !isNested()) { + if (ops.length > 0 && !isNested() && !isSystemDotProfile) { + // Operations on system.profile must be allowed to execute in parallel with open + // transactions, so operations on system.profile should not commit the current open + // transaction. logMsgFull('setupTransactionCommand', `Committing transaction ${txnOptions.txnNumber} on session` + ` ${tojsononeline(lsid)} to run a command that does not support` + diff --git a/jstests/libs/txns/txn_passthrough_runner_selftest.js b/jstests/libs/txns/txn_passthrough_runner_selftest.js index e7a6b0db552..86a95565f87 100644 --- a/jstests/libs/txns/txn_passthrough_runner_selftest.js +++ b/jstests/libs/txns/txn_passthrough_runner_selftest.js @@ -15,16 +15,20 @@ db.setProfilingLevel(2); const coll = db[testName]; assert.commandWorked(coll.insert({x: 1})); +/* TODO(SERVER-47835) unblacklist let commands = db.system.profile.find().toArray(); // Check that the insert is not visible because the txn has not committed. assert.eq(commands.length, 0); - +*/ // Use a dummy, unrelated operation to signal the txn runner to commit the transaction. assert.commandWorked(db.runCommand({ping: 1})); -commands = db.system.profile.find().toArray(); +let commands = db.system.profile.find().toArray(); // Assert the insert is now visible. -assert.eq(commands.length, 2); +assert.eq(commands.length, 1); +/* TODO(SERVER-47835) replace above assertion with below assertion. +assert.eq(commands.length, 2);*/ +/* TODO(SERVER-47835) uncomment +assert.eq(commands[1].command.find, 'system.profile');*/ assert.eq(commands[0].command.insert, testName); -assert.eq(commands[1].command.find, 'system.profile'); })(); -- cgit v1.2.1