diff options
author | Blake Oler <blake.oler@mongodb.com> | 2019-08-01 10:41:35 -0400 |
---|---|---|
committer | Blake Oler <blake.oler@mongodb.com> | 2019-08-01 16:26:43 -0400 |
commit | 927883be3ec0af1f1a8ac65176f39696a31b4962 (patch) | |
tree | 0389d7d8a8b5e051d1764a3c8d51bf9cdcbe7696 | |
parent | 42ad6f95324e0a7c04e5a10da7e1c9995dd4811f (diff) | |
download | mongo-927883be3ec0af1f1a8ac65176f39696a31b4962.tar.gz |
SERVER-42114 Allow single RS transactions to run on the config database in sharded clusters
-rw-r--r-- | buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml | 5 | ||||
-rw-r--r-- | buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml | 4 | ||||
-rw-r--r-- | buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml | 5 | ||||
-rw-r--r-- | buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml | 4 | ||||
-rw-r--r-- | buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml | 1 | ||||
-rw-r--r-- | jstests/core/txns/banned_txn_dbs_unsharded.js (renamed from jstests/core/txns/banned_txn_dbs.js) | 4 | ||||
-rw-r--r-- | jstests/sharding/banned_txn_databases_sharded.js | 78 | ||||
-rw-r--r-- | src/mongo/db/commands.cpp | 50 | ||||
-rw-r--r-- | src/mongo/db/commands.h | 7 | ||||
-rw-r--r-- | src/mongo/db/service_entry_point_common.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/transaction_validation.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/transaction_validation.h | 5 | ||||
-rw-r--r-- | src/mongo/s/commands/strategy.cpp | 6 |
13 files changed, 139 insertions, 45 deletions
diff --git a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml index 0dbfd4f8b9f..428dcaa4f73 100644 --- a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml @@ -28,10 +28,6 @@ selector: - jstests/core/txns/abort_transaction_thread_does_not_block_on_locks.js - jstests/core/txns/kill_op_on_txn_expiry.js - # Writes to the local database are not allowed through mongos. - # TODO SERVER-28756: Mongos CSRS write retry logic drops txnNumbers. - - jstests/core/txns/banned_txn_dbs.js - # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js @@ -40,6 +36,7 @@ selector: - jstests/core/txns/non_transactional_operations_on_session_with_transaction.js exclude_with_any_tags: + - assumes_against_mongod_not_mongos - does_not_support_causal_consistency # Transactions are not allowed to operate on capped collections. - requires_capped 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 17f955e261c..27890c3f68d 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 @@ -24,10 +24,6 @@ selector: - jstests/core/txns/abort_transaction_thread_does_not_block_on_locks.js - jstests/core/txns/kill_op_on_txn_expiry.js - # Writes to the local database are not allowed through mongos. - # TODO SERVER-28756: Mongos CSRS write retry logic drops txnNumbers. - - jstests/core/txns/banned_txn_dbs.js - # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml index 5d4482a396b..a39af49298e 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml @@ -28,14 +28,11 @@ selector: - jstests/core/txns/abort_transaction_thread_does_not_block_on_locks.js - jstests/core/txns/kill_op_on_txn_expiry.js - # Writes to the local database are not allowed through mongos. - # TODO SERVER-28756: Mongos CSRS write retry logic drops txnNumbers. - - jstests/core/txns/banned_txn_dbs.js - # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js exclude_with_any_tags: + - assumes_against_mongod_not_mongos # Transactions are not allowed to operate on capped collections. - requires_capped # Prepare is not a command on mongos. diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml index 88f46be33fd..8536dd79613 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml @@ -24,10 +24,6 @@ selector: - jstests/core/txns/abort_transaction_thread_does_not_block_on_locks.js - jstests/core/txns/kill_op_on_txn_expiry.js - # Writes to the local database are not allowed through mongos. - # TODO SERVER-28756: Mongos CSRS write retry logic drops txnNumbers. - - jstests/core/txns/banned_txn_dbs.js - # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml index e710caa8be2..bdd60ea3e53 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml @@ -149,6 +149,7 @@ selector: - jstests/sharding/database_and_shard_versioning_all_commands.js - jstests/sharding/shard1.js - jstests/sharding/track_unsharded_collections_create_collection.js + - jstests/sharding/banned_txn_databases_sharded.js # Enable if SERVER-41813 is backported or 4.4 becomes last-stable - jstests/sharding/invalid_system_views_sharded_collection.js diff --git a/jstests/core/txns/banned_txn_dbs.js b/jstests/core/txns/banned_txn_dbs_unsharded.js index 78bcef608a5..71d59c722cc 100644 --- a/jstests/core/txns/banned_txn_dbs.js +++ b/jstests/core/txns/banned_txn_dbs_unsharded.js @@ -1,6 +1,6 @@ // Tests that reads and writes to the config, admin, and local databases are forbidden within -// transactions. -// @tags: [uses_transactions] +// transactions on non-sharded clusters. Behavior on sharded clusters is tested separately. +// @tags: [assumes_against_mongod_not_mongos, assumes_unsharded_collection, uses_transactions] (function() { "use strict"; diff --git a/jstests/sharding/banned_txn_databases_sharded.js b/jstests/sharding/banned_txn_databases_sharded.js new file mode 100644 index 00000000000..efe36e43435 --- /dev/null +++ b/jstests/sharding/banned_txn_databases_sharded.js @@ -0,0 +1,78 @@ +/** + * Tests that: + * 1. Reads and writes to the admin database are forbidden within single replica set transactions + * on sharded clusters. + * 2. Read and writes to the config database are forbidden from mongos within single replica set + * transactions on sharded clusters. + * 3. Reads and writes to the config.transactions namespace are forbidden within single replica set + * transactions on sharded clusters, BUT read and writes to other namespaces in the config + * database are allowed. + * + * @tags: [requires_find_command] + */ + +(function() { +"use strict"; + +load("jstests/sharding/libs/sharded_transactions_helpers.js"); + +const st = new ShardingTest({shards: 1}); +const mongosSession = st.s.startSession(); +const shardSession = st.shard0.getDB("test").getMongo().startSession(); +const collName = "banned_txn_dbs"; + +jsTest.log("Verify that read and write operations within transactions are forbidden for the " + + "admin database within a transaction."); + +const adminColl = shardSession.getDatabase("admin")[collName]; + +shardSession.startTransaction(); +let error = assert.throws(() => adminColl.find().itcount()); +assert.commandFailedWithCode(error, ErrorCodes.OperationNotSupportedInTransaction); +assert.commandFailedWithCode(shardSession.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + +shardSession.startTransaction(); +assert.commandFailedWithCode(adminColl.insert({}), ErrorCodes.OperationNotSupportedInTransaction); +assert.commandFailedWithCode(shardSession.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + +jsTestLog("Verify that read and write operations within transactions are forbidden for the " + + "config database when accessed through mongos."); + +const mongosConfigDB = mongosSession.getDatabase("config"); +const clusterColls = [ + mongosConfigDB["test"], + mongosConfigDB["actionlog"], + mongosConfigDB["transaction_coords"], + mongosConfigDB["transactions"] +]; + +mongosSession.startTransaction(); +clusterColls.forEach((coll) => { + error = assert.throws(() => coll.find().itcount()); + assert.commandFailedWithCode(error, ErrorCodes.OperationNotSupportedInTransaction); +}); + +mongosSession.endSession(); + +jsTestLog("Verify that read operations within transactions work fine for the config database " + + "when not config.transactions (and directly accessed through the shard)."); + +const configDB = shardSession.getDatabase("config"); +const shardColls = [configDB["test"], configDB["actionlog"], configDB["transaction_coords"]]; + +shardSession.startTransaction(); +shardColls.forEach((coll) => { + coll.find().itcount(); +}); + +jsTestLog("Verify that read operations will not work for the config.transactions namespace."); + +const shardCollTransactions = configDB["transactions"]; +error = assert.throws(() => shardCollTransactions.find().itcount()); +assert.commandFailedWithCode(error, ErrorCodes.OperationNotSupportedInTransaction); + +shardSession.endSession(); +st.stop(); +}()); diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 3addc06942c..8c35025b156 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -431,26 +431,38 @@ bool CommandHelpers::uassertShouldAttemptParse(OperationContext* opCtx, } -Status CommandHelpers::canUseTransactions(StringData dbName, StringData cmdName) { - if (cmdName == "count"_sd) { - return {ErrorCodes::OperationNotSupportedInTransaction, - "Cannot run 'count' in a multi-document transaction. Please see " - "http://dochub.mongodb.org/core/transaction-count for a recommended alternative."}; - } - - if (txnCmdWhitelist.find(cmdName) == txnCmdWhitelist.cend()) { - return {ErrorCodes::OperationNotSupportedInTransaction, - str::stream() << "Cannot run '" << cmdName << "' in a multi-document transaction."}; - } - - if (dbName == "config"_sd || dbName == "local"_sd || - (dbName == "admin"_sd && txnAdminCommands.find(cmdName) == txnAdminCommands.cend())) { - return {ErrorCodes::OperationNotSupportedInTransaction, - str::stream() << "Cannot run command against the '" << dbName - << "' database in a transaction"}; +void CommandHelpers::canUseTransactions(const NamespaceString& nss, + StringData cmdName, + bool allowTransactionsOnConfigDatabase) { + + uassert(ErrorCodes::OperationNotSupportedInTransaction, + "Cannot run 'count' in a multi-document transaction. Please see " + "http://dochub.mongodb.org/core/transaction-count for a recommended alternative.", + cmdName != "count"_sd); + + uassert(ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Cannot run '" << cmdName << "' in a multi-document transaction.", + txnCmdWhitelist.find(cmdName) != txnCmdWhitelist.cend()); + + const auto dbName = nss.db(); + + uassert(ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Cannot run command against the '" << dbName + << "' database in a transaction.", + dbName != NamespaceString::kLocalDb && + (dbName != NamespaceString::kAdminDb || + txnAdminCommands.find(cmdName) != txnAdminCommands.cend())); + + if (allowTransactionsOnConfigDatabase) { + uassert(ErrorCodes::OperationNotSupportedInTransaction, + "Cannot run command against the config.transactions namespace in a transaction" + "on a sharded cluster.", + nss != NamespaceString::kSessionTransactionsTableNamespace); + } else { + uassert(ErrorCodes::OperationNotSupportedInTransaction, + "Cannot run command against the config database in a transaction.", + dbName != "config"_sd); } - - return Status::OK(); } constexpr StringData CommandHelpers::kHelpFieldName; diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index 3039411660a..0d6b020d757 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -211,9 +211,12 @@ struct CommandHelpers { const OpMsgRequest& request); /** - * Returns OK if command is allowed to run under a transaction in the given database. + * Verifies that command is allowed to run under a transaction in the given database or + * namespace, and throws if that verification doesn't pass. */ - static Status canUseTransactions(StringData dbName, StringData cmdName); + static void canUseTransactions(const NamespaceString& nss, + StringData cmdName, + bool allowTransactionsOnConfigDatabase); static constexpr StringData kHelpFieldName = "help"_sd; diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 39343d97e39..82ae8328337 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -664,7 +664,15 @@ void execCommandDatabase(OperationContext* opCtx, str::stream() << "Invalid database name: '" << dbname << "'", NamespaceString::validDBName(dbname, NamespaceString::DollarInDbNameBehavior::Allow)); - validateSessionOptions(sessionOptions, command->getName(), dbname); + + const auto allowTransactionsOnConfigDatabase = + (serverGlobalParams.clusterRole == ClusterRole::ConfigServer || + serverGlobalParams.clusterRole == ClusterRole::ShardServer); + + validateSessionOptions(sessionOptions, + command->getName(), + invocation->ns(), + allowTransactionsOnConfigDatabase); std::unique_ptr<MaintenanceModeSetter> mmSetter; diff --git a/src/mongo/db/transaction_validation.cpp b/src/mongo/db/transaction_validation.cpp index a2af2228612..7e0af2d4a5b 100644 --- a/src/mongo/db/transaction_validation.cpp +++ b/src/mongo/db/transaction_validation.cpp @@ -70,9 +70,10 @@ bool shouldCommandSkipSessionCheckout(StringData cmdName) { void validateSessionOptions(const OperationSessionInfoFromClient& sessionOptions, StringData cmdName, - StringData dbname) { + const NamespaceString& nss, + bool allowTransactionsOnConfigDatabase) { if (sessionOptions.getAutocommit()) { - uassertStatusOK(CommandHelpers::canUseTransactions(dbname, cmdName)); + CommandHelpers::canUseTransactions(nss, cmdName, allowTransactionsOnConfigDatabase); } if (!sessionOptions.getAutocommit() && sessionOptions.getTxnNumber()) { diff --git a/src/mongo/db/transaction_validation.h b/src/mongo/db/transaction_validation.h index 85895a4788d..ec22c7f304e 100644 --- a/src/mongo/db/transaction_validation.h +++ b/src/mongo/db/transaction_validation.h @@ -46,10 +46,11 @@ void validateWriteConcernForTransaction(const WriteConcernOptions& wcResult, Str bool shouldCommandSkipSessionCheckout(StringData cmdName); /** - * Throws if the given session options are invalid for the given command and target database. + * Throws if the given session options are invalid for the given command and target namespace. */ void validateSessionOptions(const OperationSessionInfoFromClient& sessionOptions, StringData cmdName, - StringData dbname); + const NamespaceString& nss, + bool allowTransactionsOnConfigDatabase); } // namespace mongo diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index b917da533c1..f059f195531 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -382,7 +382,11 @@ void runCommand(OperationContext* opCtx, command->attachLogicalSessionsToOpCtx(), true, true); - validateSessionOptions(osi, command->getName(), nss.db()); + + // TODO SERVER-28756: Change allowTransactionsOnConfigDatabase to true once we fix the bug + // where the mongos custom write path incorrectly drops the client's txnNumber. + auto allowTransactionsOnConfigDatabase = false; + validateSessionOptions(osi, command->getName(), nss, allowTransactionsOnConfigDatabase); auto& readConcernArgs = repl::ReadConcernArgs::get(opCtx); auto readConcernParseStatus = [&]() { |