diff options
30 files changed, 181 insertions, 332 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 0e3291d04c4..26ea2a0b9cc 100644 --- a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml @@ -14,9 +14,6 @@ selector: # No featureCompatibilityVersion parameter on mongos. - jstests/core/txns/abort_unprepared_transactions_on_FCV_downgrade.js - # Mongos doesn't upconvert from local or majority level readConcern to snapshot. - - jstests/core/txns/upconvert_read_concern.js - # Implicitly creates a database through a collection rename, which does not work in a sharded # cluster. - jstests/core/txns/transactions_block_ddl.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 a41dd188ab3..5963060e8cb 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 @@ -10,9 +10,6 @@ selector: # No featureCompatibilityVersion parameter on mongos. - jstests/core/txns/abort_unprepared_transactions_on_FCV_downgrade.js - # Mongos doesn't upconvert from local or majority level readConcern to snapshot. - - jstests/core/txns/upconvert_read_concern.js - # Implicitly creates a database through a collection rename, which does not work in a sharded # cluster. - jstests/core/txns/transactions_block_ddl.js diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml index d7b79221910..3602c55a8f8 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml @@ -14,9 +14,6 @@ selector: # No featureCompatibilityVersion parameter on mongos. - jstests/core/txns/abort_unprepared_transactions_on_FCV_downgrade.js - # Mongos doesn't upconvert from local or majority level readConcern to snapshot. - - jstests/core/txns/upconvert_read_concern.js - # Implicitly creates a database through a collection rename, which does not work in a sharded # cluster. - jstests/core/txns/transactions_block_ddl.js diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml index f8dcd1e6bd4..b7ac7337599 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml @@ -10,9 +10,6 @@ selector: # No featureCompatibilityVersion parameter on mongos. - jstests/core/txns/abort_unprepared_transactions_on_FCV_downgrade.js - # Mongos doesn't upconvert from local or majority level readConcern to snapshot. - - jstests/core/txns/upconvert_read_concern.js - # Implicitly creates a database through a collection rename, which does not work in a sharded # cluster. - jstests/core/txns/transactions_block_ddl.js diff --git a/jstests/core/txns/upconvert_read_concern.js b/jstests/core/txns/read_concern.js index 2f49280e128..73b87841f26 100644 --- a/jstests/core/txns/upconvert_read_concern.js +++ b/jstests/core/txns/read_concern.js @@ -1,11 +1,12 @@ -// Test that the readConcern specified on a multi-statement transaction is upconverted to have level -// 'snapshot'. -// @tags: [uses_transactions] +// Test that no matter the readConcern specified on a multi-statement transaction, it has snapshot +// isolation. Also test that readConcern linearizable and available are not allowed with a +// transaction. +// @tags: [uses_transactions, requires_majority_read_concern] (function() { "use strict"; const dbName = "test"; -const collName = "upconvert_read_concern"; +const collName = "read_concern"; const testDB = db.getSiblingDB(dbName); const testColl = testDB[collName]; @@ -19,8 +20,10 @@ const sessionOptions = { const session = db.getMongo().startSession(sessionOptions); const sessionDb = session.getDatabase(dbName); -function testUpconvertReadConcern(readConcern) { - jsTest.log("Test that the following readConcern is upconverted: " + tojson(readConcern)); +function testReadConcernMaintainsSnapshotIsolationInTransaction(readConcern) { + jsTest.log( + "Test that the following multi-document transaction has snapshot isolation with readConcern: " + + tojson(readConcern)); assert.commandWorked(testColl.remove({}, {writeConcern: {w: "majority"}})); // Start a new transaction with the given readConcern. @@ -42,14 +45,16 @@ function testUpconvertReadConcern(readConcern) { assert.commandWorked(session.commitTransaction_forTesting()); } -testUpconvertReadConcern(null); -testUpconvertReadConcern({}); -testUpconvertReadConcern({level: "local"}); -testUpconvertReadConcern({level: "majority"}); -testUpconvertReadConcern({level: "snapshot"}); +testReadConcernMaintainsSnapshotIsolationInTransaction(null); +testReadConcernMaintainsSnapshotIsolationInTransaction({}); +testReadConcernMaintainsSnapshotIsolationInTransaction({level: "local"}); +testReadConcernMaintainsSnapshotIsolationInTransaction({level: "majority"}); +testReadConcernMaintainsSnapshotIsolationInTransaction({level: "snapshot"}); -function testCannotUpconvertReadConcern(readConcern) { - jsTest.log("Test that the following readConcern cannot be upconverted: " + readConcern); +function testReadConcernNotSupportedInTransaction(readConcern) { + jsTest.log( + "Test that the following readConcern is not supported in a multi-document transaction: " + + readConcern); // Start a new transaction with the given readConcern. session.startTransaction(); @@ -63,8 +68,8 @@ function testCannotUpconvertReadConcern(readConcern) { ErrorCodes.NoSuchTransaction); } -testCannotUpconvertReadConcern({level: "available"}); -testCannotUpconvertReadConcern({level: "linearizable"}); +testReadConcernNotSupportedInTransaction({level: "available"}); +testReadConcernNotSupportedInTransaction({level: "linearizable"}); jsTest.log("Test starting a transaction with an invalid readConcern"); @@ -91,5 +96,19 @@ assert.commandFailedWithCode( // The transaction is still active and can be committed. assert.commandWorked(session.commitTransaction_forTesting()); +jsTest.log("Test specifying non-snapshot readConcern on the second statement in a transaction"); + +// Start a new transaction with majority readConcern. +session.startTransaction(); +assert.commandWorked(sessionDb.runCommand({find: collName, readConcern: {level: "majority"}})); + +// The second statement cannot specify a readConcern. +assert.commandFailedWithCode( + sessionDb.runCommand({find: collName, readConcern: {level: "majority"}}), + ErrorCodes.InvalidOptions); + +// The transaction is still active and can be committed. +assert.commandWorked(session.commitTransaction_forTesting()); + session.endSession(); -}()); +}());
\ No newline at end of file diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index 35ac31fcddb..f355df6289c 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -472,16 +472,9 @@ public: virtual bool supportsWriteConcern() const = 0; /** - * Returns true if this Command supports the given readConcern level. Takes the command object - * and the name of the database on which it was invoked as arguments, so that readConcern can be - * conditionally rejected based on the command's parameters and/or namespace. - * - * 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. - * - * 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. + * Returns true if this command invocation supports the given readConcern level. 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. */ virtual bool supportsReadConcern(repl::ReadConcernLevel level) const { return level == repl::ReadConcernLevel::kLocalReadConcern; @@ -623,6 +616,10 @@ 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/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index b3fd2d6c8bc..da7b522855b 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -93,7 +93,7 @@ public: bool supportsReadConcern(const std::string& dbName, const BSONObj& cmdObj, repl::ReadConcernLevel level) const override { - return level != repl::ReadConcernLevel::kSnapshotReadConcern; + return true; } ReadWriteType getReadWriteType() const override { diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index 0a30fd49293..b1b169988da 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -220,13 +220,6 @@ public: return AllowedOnSecondary::kNever; } - bool supportsReadConcern(const std::string& dbName, - const BSONObj& cmdObj, - repl::ReadConcernLevel level) const final { - return level == repl::ReadConcernLevel::kLocalReadConcern || - level == repl::ReadConcernLevel::kSnapshotReadConcern; - } - bool supportsWriteConcern(const BSONObj& cmd) const override { return true; } diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp index 6f91ec1fca7..c146024162f 100644 --- a/src/mongo/db/commands/getmore_cmd.cpp +++ b/src/mongo/db/commands/getmore_cmd.cpp @@ -131,9 +131,11 @@ void validateTxnNumber(OperationContext* opCtx, void applyCursorReadConcern(OperationContext* opCtx, repl::ReadConcernArgs rcArgs) { const auto replicationMode = repl::ReplicationCoordinator::get(opCtx)->getReplicationMode(); - // Select the appropriate read source. + // Select the appropriate read source. If we are in a transaction with read concern majority, + // this will already be set to kNoTimestamp, so don't set it again. if (replicationMode == repl::ReplicationCoordinator::modeReplSet && - rcArgs.getLevel() == repl::ReadConcernLevel::kMajorityReadConcern) { + rcArgs.getLevel() == repl::ReadConcernLevel::kMajorityReadConcern && + !opCtx->inMultiDocumentTransaction()) { switch (rcArgs.getMajorityReadMechanism()) { case repl::ReadConcernArgs::MajorityReadMechanism::kMajoritySnapshot: { // Make sure we read from the majority snapshot. diff --git a/src/mongo/db/commands/killcursors_cmd.cpp b/src/mongo/db/commands/killcursors_cmd.cpp index a81c59b8702..591a381106c 100644 --- a/src/mongo/db/commands/killcursors_cmd.cpp +++ b/src/mongo/db/commands/killcursors_cmd.cpp @@ -48,14 +48,6 @@ class KillCursorsCmd final : public KillCursorsCmdBase { public: KillCursorsCmd() = default; - bool supportsReadConcern(const std::string& dbName, - const BSONObj& cmdObj, - repl::ReadConcernLevel level) const final { - // killCursors must support snapshot read concern in order to be run in transactions. - return level == repl::ReadConcernLevel::kLocalReadConcern || - level == repl::ReadConcernLevel::kSnapshotReadConcern; - } - bool run(OperationContext* opCtx, const std::string& dbname, const BSONObj& cmdObj, diff --git a/src/mongo/db/commands/run_aggregate.cpp b/src/mongo/db/commands/run_aggregate.cpp index bf6b65c13f4..5fc6cadacb3 100644 --- a/src/mongo/db/commands/run_aggregate.cpp +++ b/src/mongo/db/commands/run_aggregate.cpp @@ -499,20 +499,16 @@ Status runAggregate(OperationContext* opCtx, { const LiteParsedPipeline liteParsedPipeline(request); - try { - // Check whether the parsed pipeline supports the given read concern. - liteParsedPipeline.assertSupportsReadConcern( - opCtx, request.getExplain(), serverGlobalParams.enableMajorityReadConcern); - } catch (const DBException& ex) { - // If we are in a multi-document transaction, we intercept the 'readConcern' - // assertion in order to provide a more descriptive error message and code. - if (opCtx->inMultiDocumentTransaction()) { - return {ErrorCodes::OperationNotSupportedInTransaction, - ex.toStatus("Operation not permitted in transaction").reason()}; - } - return ex.toStatus(); + // If we are in a transaction, check whether the parsed pipeline supports + // being in a transaction. + if (opCtx->inMultiDocumentTransaction()) { + liteParsedPipeline.assertSupportsMultiDocumentTransaction(request.getExplain()); } + // Check whether the parsed pipeline supports the given read concern. + liteParsedPipeline.assertSupportsReadConcern( + opCtx, request.getExplain(), serverGlobalParams.enableMajorityReadConcern); + const auto& pipelineInvolvedNamespaces = liteParsedPipeline.getInvolvedNamespaces(); // If this is a collectionless aggregation, we won't create 'ctx' but will still need an diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index e5b6d25396b..8d7339d1d80 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -242,11 +242,6 @@ private: } } - bool supportsReadConcern(repl::ReadConcernLevel level) const final { - return level == repl::ReadConcernLevel::kLocalReadConcern || - level == repl::ReadConcernLevel::kSnapshotReadConcern; - } - bool supportsWriteConcern() const final { return true; } diff --git a/src/mongo/db/pipeline/document_source_change_stream.h b/src/mongo/db/pipeline/document_source_change_stream.h index 70357cd433e..182a9373e12 100644 --- a/src/mongo/db/pipeline/document_source_change_stream.h +++ b/src/mongo/db/pipeline/document_source_change_stream.h @@ -82,13 +82,17 @@ public: void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const { // Only "majority" is allowed for change streams. - uassert(ErrorCodes::InvalidOptions, - str::stream() << "$changeStream cannot run with a readConcern other than " - << "'majority', or in a multi-document transaction. Current " - "readConcern: " - << readConcern.toString(), - !readConcern.hasLevel() || - readConcern.getLevel() == repl::ReadConcernLevel::kMajorityReadConcern); + uassert( + ErrorCodes::InvalidOptions, + str::stream() + << "$changeStream cannot run with a readConcern other than 'majority'. Current " + << "readConcern: " << readConcern.toString(), + !readConcern.hasLevel() || + readConcern.getLevel() == repl::ReadConcernLevel::kMajorityReadConcern); + } + + void assertSupportsMultiDocumentTransaction() const { + transactionNotSupported(kStageName); } private: diff --git a/src/mongo/db/pipeline/document_source_current_op.h b/src/mongo/db/pipeline/document_source_current_op.h index fffc51503a3..bc66e0fef64 100644 --- a/src/mongo/db/pipeline/document_source_current_op.h +++ b/src/mongo/db/pipeline/document_source_current_op.h @@ -80,11 +80,11 @@ public: } void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const { - uassert(ErrorCodes::InvalidOptions, - str::stream() << "Aggregation stage " << kStageName << " cannot run with a " - << "readConcern other than 'local', or in a multi-document " - << "transaction. Current readConcern: " << readConcern.toString(), - readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); + onlyReadConcernLocalSupported(kStageName, readConcern); + } + + void assertSupportsMultiDocumentTransaction() const { + transactionNotSupported(kStageName); } private: diff --git a/src/mongo/db/pipeline/document_source_list_cached_and_active_users.h b/src/mongo/db/pipeline/document_source_list_cached_and_active_users.h index 3f297e25a49..9406bf2e126 100644 --- a/src/mongo/db/pipeline/document_source_list_cached_and_active_users.h +++ b/src/mongo/db/pipeline/document_source_list_cached_and_active_users.h @@ -70,11 +70,11 @@ public: } void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const { - uassert(ErrorCodes::InvalidOptions, - str::stream() << "Aggregation stage " << kStageName << " cannot run with a " - << "readConcern other than 'local', or in a multi-document " - << "transaction. Current readConcern: " << readConcern.toString(), - readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); + onlyReadConcernLocalSupported(kStageName, readConcern); + } + + void assertSupportsMultiDocumentTransaction() const { + transactionNotSupported(kStageName); } }; diff --git a/src/mongo/db/pipeline/document_source_list_local_sessions.h b/src/mongo/db/pipeline/document_source_list_local_sessions.h index 0cc7ba1a929..b4439a4323d 100644 --- a/src/mongo/db/pipeline/document_source_list_local_sessions.h +++ b/src/mongo/db/pipeline/document_source_list_local_sessions.h @@ -82,13 +82,11 @@ public: } void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const { - uassert(ErrorCodes::InvalidOptions, - str::stream() << "Aggregation stage " - << DocumentSourceListLocalSessions::kStageName - << " cannot run with a " - << "readConcern other than 'local', or in a multi-document " - << "transaction. Current readConcern: " << readConcern.toString(), - readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); + onlyReadConcernLocalSupported(DocumentSourceListLocalSessions::kStageName, readConcern); + } + + void assertSupportsMultiDocumentTransaction() const { + transactionNotSupported(DocumentSourceListLocalSessions::kStageName); } private: diff --git a/src/mongo/db/pipeline/document_source_plan_cache_stats.h b/src/mongo/db/pipeline/document_source_plan_cache_stats.h index f0cf248ae5b..5902896ee72 100644 --- a/src/mongo/db/pipeline/document_source_plan_cache_stats.h +++ b/src/mongo/db/pipeline/document_source_plan_cache_stats.h @@ -66,12 +66,11 @@ public: } void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const { - uassert(ErrorCodes::InvalidOptions, - str::stream() << "Aggregation stage " - << DocumentSourcePlanCacheStats::kStageName - << " requires read concern local but found " - << readConcern.toString(), - readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); + onlyReadConcernLocalSupported(DocumentSourcePlanCacheStats::kStageName, readConcern); + } + + void assertSupportsMultiDocumentTransaction() const { + transactionNotSupported(DocumentSourcePlanCacheStats::kStageName); } private: diff --git a/src/mongo/db/pipeline/lite_parsed_document_source.h b/src/mongo/db/pipeline/lite_parsed_document_source.h index 883d68cff91..064e22389f3 100644 --- a/src/mongo/db/pipeline/lite_parsed_document_source.h +++ b/src/mongo/db/pipeline/lite_parsed_document_source.h @@ -134,6 +134,31 @@ public: * UserException if not compatible. */ virtual void assertSupportsReadConcern(const repl::ReadConcernArgs& readConcern) const {} + + /** + * Verifies that this stage is allowed to run in a multi-document transaction. Throws a + * UserException if not compatible. This should only be called if the caller has determined the + * current operation is part of a transaction. + */ + virtual void assertSupportsMultiDocumentTransaction() const {} + +protected: + void transactionNotSupported(StringData stageName) const { + uasserted(ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Operation not permitted in transaction :: caused by :: " + << "Aggregation stage " << stageName << " cannot run within a " + << "multi-document transaction."); + } + + void onlyReadConcernLocalSupported(StringData stageName, + const repl::ReadConcernArgs& readConcern) const { + uassert(ErrorCodes::InvalidOptions, + str::stream() + << "Aggregation stage " << stageName + << " cannot run with a readConcern other than 'local'. Current readConcern: " + << readConcern.toString(), + readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); + } }; class LiteParsedDocumentSourceDefault final : public LiteParsedDocumentSource { diff --git a/src/mongo/db/pipeline/lite_parsed_pipeline.cpp b/src/mongo/db/pipeline/lite_parsed_pipeline.cpp index 81a10467a58..0b9909c5ba4 100644 --- a/src/mongo/db/pipeline/lite_parsed_pipeline.cpp +++ b/src/mongo/db/pipeline/lite_parsed_pipeline.cpp @@ -38,7 +38,7 @@ namespace mongo { void LiteParsedPipeline::assertSupportsReadConcern( OperationContext* opCtx, boost::optional<ExplainOptions::Verbosity> explain, - bool enableMajorityReadConcern) const try { + bool enableMajorityReadConcern) const { auto readConcern = repl::ReadConcernArgs::get(opCtx); // Reject non change stream aggregation queries that try to use "majority" read concern when @@ -53,26 +53,24 @@ void LiteParsedPipeline::assertSupportsReadConcern( uassert(ErrorCodes::InvalidOptions, str::stream() << "Explain for the aggregate command cannot run with a readConcern " - << "other than 'local', or in a multi-document transaction. Current " - << "readConcern: " << readConcern.toString(), + << "other than 'local'. Current readConcern: " << readConcern.toString(), !explain || readConcern.getLevel() == repl::ReadConcernLevel::kLocalReadConcern); for (auto&& spec : _stageSpecs) { spec->assertSupportsReadConcern(readConcern); } -} catch (const DBException& ex) { - using namespace std::string_literals; - // If we are in a multi-document transaction, we intercept the 'readConcern' - // assertion in order to provide a more descriptive error message and code. +} + +void LiteParsedPipeline::assertSupportsMultiDocumentTransaction( + boost::optional<ExplainOptions::Verbosity> explain) const { uassert(ErrorCodes::OperationNotSupportedInTransaction, - "Operation not permitted in transaction"s - " :: caused by :: "s + - ex.reason(), - // This will be true for retryable writes in addition to the intended case of an in- - // process transaction. However the retryable write code path should not call this - // function. - !opCtx->getTxnNumber()); - throw; + "Operation not permitted in transaction :: caused by :: Explain for the aggregate " + "command cannot run within a multi-document transaction", + !explain); + + for (auto&& spec : _stageSpecs) { + spec->assertSupportsMultiDocumentTransaction(); + } } bool LiteParsedPipeline::verifyIsSupported( @@ -80,6 +78,10 @@ bool LiteParsedPipeline::verifyIsSupported( const std::function<bool(OperationContext*, const NamespaceString&)> isSharded, const boost::optional<ExplainOptions::Verbosity> explain, bool enableMajorityReadConcern) const { + // Verify litePipe can be run in a transaction. + if (opCtx->inMultiDocumentTransaction()) { + assertSupportsMultiDocumentTransaction(explain); + } // Verify litePipe can be run at the given read concern. assertSupportsReadConcern(opCtx, explain, enableMajorityReadConcern); // Verify that no involved namespace is sharded unless allowed by the pipeline. diff --git a/src/mongo/db/pipeline/lite_parsed_pipeline.h b/src/mongo/db/pipeline/lite_parsed_pipeline.h index 7fa83fced43..f5578873621 100644 --- a/src/mongo/db/pipeline/lite_parsed_pipeline.h +++ b/src/mongo/db/pipeline/lite_parsed_pipeline.h @@ -130,6 +130,14 @@ public: bool enableMajorityReadConcern) const; /** + * Verifies that this pipeline is allowed to run in a multi-document transaction. This ensures + * that each stage is compatible, and throws a UserException if not. This should only be called + * if the caller has determined the current operation is part of a transaction. + */ + void assertSupportsMultiDocumentTransaction( + boost::optional<ExplainOptions::Verbosity> explain) const; + + /** * Perform checks that verify that the LitePipe is valid. Note that this function must be called * before forwarding an aggregation command on an unsharded collection, in order to verify that * the involved namespaces are allowed to be sharded. Returns true if any involved namespace is diff --git a/src/mongo/db/read_concern_mongod.cpp b/src/mongo/db/read_concern_mongod.cpp index 60028d779f9..8207e58fed4 100644 --- a/src/mongo/db/read_concern_mongod.cpp +++ b/src/mongo/db/read_concern_mongod.cpp @@ -207,8 +207,14 @@ Status makeNoopWriteIfNeeded(OperationContext* opCtx, LogicalTime clusterTime) { /** * Evaluates if it's safe for the command to ignore prepare conflicts. */ -bool canIgnorePrepareConflicts(const repl::ReadConcernArgs& readConcernArgs) { +bool canIgnorePrepareConflicts(OperationContext* opCtx, + const repl::ReadConcernArgs& readConcernArgs) { + if (opCtx->inMultiDocumentTransaction()) { + return false; + } + auto readConcernLevel = readConcernArgs.getLevel(); + // Only these read concern levels are eligible for ignoring prepare conflicts. if (readConcernLevel != repl::ReadConcernLevel::kLocalReadConcern && readConcernLevel != repl::ReadConcernLevel::kAvailableReadConcern && @@ -240,7 +246,7 @@ MONGO_REGISTER_SHIM(setPrepareConflictBehaviorForReadConcern) // Enforce prepare conflict behavior if the command is not eligible to ignore prepare conflicts. if (!(prepareConflictBehavior == PrepareConflictBehavior::kEnforce || - canIgnorePrepareConflicts(readConcernArgs))) { + canIgnorePrepareConflicts(opCtx, readConcernArgs))) { prepareConflictBehavior = PrepareConflictBehavior::kEnforce; } diff --git a/src/mongo/db/repl/read_concern_args.cpp b/src/mongo/db/repl/read_concern_args.cpp index 5ec5ae968c1..bb060b5a4dc 100644 --- a/src/mongo/db/repl/read_concern_args.cpp +++ b/src/mongo/db/repl/read_concern_args.cpp @@ -105,19 +105,10 @@ ReadConcernLevel ReadConcernArgs::getLevel() const { return _level.value_or(ReadConcernLevel::kLocalReadConcern); } -ReadConcernLevel ReadConcernArgs::getOriginalLevel() const { - // If no read concern specified, default to "local" - return _originalLevel.value_or(ReadConcernLevel::kLocalReadConcern); -} - bool ReadConcernArgs::hasLevel() const { return _level.is_initialized(); } -bool ReadConcernArgs::hasOriginalLevel() const { - return _originalLevel.is_initialized(); -} - boost::optional<OpTime> ReadConcernArgs::getArgsOpTime() const { return _opTime; } @@ -198,7 +189,6 @@ Status ReadConcernArgs::initialize(const BSONElement& readConcernElem) { << " must be either 'local', 'majority', " "'linearizable', 'available', or 'snapshot'"); } - _originalLevel = _level; } else { return Status(ErrorCodes::InvalidOptions, str::stream() << "Unrecognized option in " << kReadConcernFieldName @@ -273,26 +263,6 @@ bool ReadConcernArgs::isSpeculativeMajority() const { _majorityReadMechanism == MajorityReadMechanism::kSpeculative; } -Status ReadConcernArgs::upconvertReadConcernLevelToSnapshot() { - if (_level && *_level != ReadConcernLevel::kSnapshotReadConcern && - *_level != ReadConcernLevel::kMajorityReadConcern && - *_level != ReadConcernLevel::kLocalReadConcern) { - return Status(ErrorCodes::InvalidOptions, - "The readConcern level must be either 'local' or 'majority' in order to " - "upconvert the readConcern level to 'snapshot'"); - } - - if (_opTime) { - return Status(ErrorCodes::InvalidOptions, - str::stream() << "Cannot upconvert the readConcern level to 'snapshot' when '" - << kAfterOpTimeFieldName << "' is provided"); - } - - _originalLevel = _level; - _level = ReadConcernLevel::kSnapshotReadConcern; - return Status::OK(); -} - void ReadConcernArgs::appendInfo(BSONObjBuilder* builder) const { BSONObjBuilder rcBuilder(builder->subobjStart(kReadConcernFieldName)); diff --git a/src/mongo/db/repl/read_concern_args.h b/src/mongo/db/repl/read_concern_args.h index b4307fc94c3..775877f9b34 100644 --- a/src/mongo/db/repl/read_concern_args.h +++ b/src/mongo/db/repl/read_concern_args.h @@ -104,12 +104,6 @@ public: Status initialize(const BSONElement& readConcernElem); /** - * Upconverts the readConcern level to 'snapshot', or returns a non-ok status if this - * readConcern cannot be upconverted. - */ - Status upconvertReadConcernLevelToSnapshot(); - - /** * Sets the mechanism we should use to satisfy 'majority' reads. * * Invalid to call unless the read concern level is 'kMajorityReadConcern'. @@ -143,23 +137,12 @@ public: * Returns default kLocalReadConcern if _level is not set. */ ReadConcernLevel getLevel() const; - - /** - * Returns readConcernLevel before upconverting, or same as getLevel() if not upconverted. - */ - ReadConcernLevel getOriginalLevel() const; - /** * Checks whether _level is explicitly set. */ bool hasLevel() const; /** - * Checks whether _originalLevel is explicitly set. - */ - bool hasOriginalLevel() const; - - /** * Returns the opTime. Deprecated: will be replaced with getArgsAfterClusterTime. */ boost::optional<OpTime> getArgsOpTime() const; @@ -187,11 +170,6 @@ private: boost::optional<ReadConcernLevel> _level; /** - * If the read concern was upconverted, the original read concern level. - */ - boost::optional<ReadConcernLevel> _originalLevel; - - /** * The mechanism to use for satisfying 'majority' reads. Only meaningful if the read concern * level is 'majority'. */ diff --git a/src/mongo/db/repl/read_concern_args_test.cpp b/src/mongo/db/repl/read_concern_args_test.cpp index d6907a31f26..036e0fbd086 100644 --- a/src/mongo/db/repl/read_concern_args_test.cpp +++ b/src/mongo/db/repl/read_concern_args_test.cpp @@ -467,129 +467,6 @@ TEST(ReadAfterSerialize, AtClusterTimeAndLevelSnapshot) { ASSERT_BSONOBJ_EQ(expectedObj, builder.done()); } -TEST(UpconvertReadConcernLevelToSnapshot, EmptyLevel) { - ReadConcernArgs readConcern; - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getLevel()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelLocal) { - ReadConcernArgs readConcern; - ASSERT_OK(readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName << "local")))); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getLevel()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelMajority) { - ReadConcernArgs readConcern; - ASSERT_OK( - readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName << "majority")))); - ASSERT(ReadConcernLevel::kMajorityReadConcern == readConcern.getLevel()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kMajorityReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelSnapshot) { - ReadConcernArgs readConcern; - ASSERT_OK( - readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName << "snapshot")))); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelSnapshotWithAtClusterTime) { - ReadConcernArgs readConcern; - auto atClusterTime = LogicalTime(Timestamp(20, 30)); - ASSERT_OK(readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName - << "snapshot" - << ReadConcernArgs::kAtClusterTimeFieldName - << atClusterTime.asTimestamp())))); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT_TRUE(readConcern.getArgsAtClusterTime()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getOriginalLevel()); - ASSERT_TRUE(readConcern.getArgsAtClusterTime()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, AfterClusterTime) { - ReadConcernArgs readConcern; - auto afterClusterTime = LogicalTime(Timestamp(20, 30)); - ASSERT_OK(readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kAfterClusterTimeFieldName - << afterClusterTime.asTimestamp())))); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getLevel()); - ASSERT_TRUE(readConcern.getArgsAfterClusterTime()); - - ASSERT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kSnapshotReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getOriginalLevel()); - ASSERT_TRUE(readConcern.getArgsAfterClusterTime()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelAvailable) { - ReadConcernArgs readConcern; - ASSERT_OK( - readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName << "available")))); - ASSERT(ReadConcernLevel::kAvailableReadConcern == readConcern.getLevel()); - - ASSERT_NOT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kAvailableReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kAvailableReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, LevelLinearizable) { - ReadConcernArgs readConcern; - ASSERT_OK( - readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kLevelFieldName << "linearizable")))); - ASSERT(ReadConcernLevel::kLinearizableReadConcern == readConcern.getLevel()); - - ASSERT_NOT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kLinearizableReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kLinearizableReadConcern == readConcern.getOriginalLevel()); -} - -TEST(UpconvertReadConcernLevelToSnapshot, AfterOpTime) { - ReadConcernArgs readConcern; - ASSERT_OK(readConcern.initialize(BSON("find" - << "test" << ReadConcernArgs::kReadConcernFieldName - << BSON(ReadConcernArgs::kAfterOpTimeFieldName - << BSON(OpTime::kTimestampFieldName - << Timestamp(20, 30) - << OpTime::kTermFieldName << 2))))); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getLevel()); - ASSERT_TRUE(readConcern.getArgsOpTime()); - - ASSERT_NOT_OK(readConcern.upconvertReadConcernLevelToSnapshot()); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getLevel()); - ASSERT(ReadConcernLevel::kLocalReadConcern == readConcern.getOriginalLevel()); - ASSERT_TRUE(readConcern.getArgsOpTime()); -} } // unnamed namespace } // namespace repl diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index e685825f7ef..9fba9cdf02f 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -1510,7 +1510,7 @@ Status ReplicationCoordinatorImpl::_waitUntilClusterTimeForRead(OperationContext auto targetOpTime = OpTime(clusterTime.asTimestamp(), OpTime::kUninitializedTerm); invariant(!readConcern.getArgsOpTime()); - // We don't set isMajorityCommittedRead for kSnapshotReadConcern because snapshots are always + // We don't set isMajorityCommittedRead for transactions because snapshots are always // speculative; we wait for majority when the transaction commits. // // Speculative majority reads do not need to wait for the commit point to advance to satisfy @@ -1520,7 +1520,7 @@ Status ReplicationCoordinatorImpl::_waitUntilClusterTimeForRead(OperationContext // durability guarantee. const bool isMajorityCommittedRead = readConcern.getLevel() == ReadConcernLevel::kMajorityReadConcern && - !readConcern.isSpeculativeMajority(); + !readConcern.isSpeculativeMajority() && !opCtx->inMultiDocumentTransaction(); return _waitUntilOpTime(opCtx, isMajorityCommittedRead, targetOpTime, deadline); } diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 2327a460410..26b4c75ca18 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -227,7 +227,7 @@ private: */ StatusWith<repl::ReadConcernArgs> _extractReadConcern(const CommandInvocation* invocation, const BSONObj& cmdObj, - bool upconvertToSnapshot) { + bool startTransaction) { repl::ReadConcernArgs readConcernArgs; auto readConcernParseStatus = readConcernArgs.initialize(cmdObj); @@ -235,27 +235,32 @@ StatusWith<repl::ReadConcernArgs> _extractReadConcern(const CommandInvocation* i return readConcernParseStatus; } - if (upconvertToSnapshot) { - auto upconvertToSnapshotStatus = readConcernArgs.upconvertReadConcernLevelToSnapshot(); - if (!upconvertToSnapshotStatus.isOK()) { - return upconvertToSnapshotStatus; - } + auto readConcernLevel = readConcernArgs.getLevel(); + if (startTransaction && readConcernLevel != repl::ReadConcernLevel::kSnapshotReadConcern && + readConcernLevel != repl::ReadConcernLevel::kMajorityReadConcern && + readConcernLevel != repl::ReadConcernLevel::kLocalReadConcern) { + return Status( + ErrorCodes::InvalidOptions, + "The readConcern level must be either 'local' (default), 'majority' or 'snapshot' in " + "order to run in a transaction"); } - if (!invocation->supportsReadConcern(readConcernArgs.getLevel())) { - // We must be in a transaction if the readConcern level was upconverted to snapshot and the - // command must support readConcern level snapshot in order to be supported in transactions. - if (upconvertToSnapshot) { - return { - ErrorCodes::OperationNotSupportedInTransaction, - str::stream() << "Command is not supported as the first command in a transaction"}; - } + if (startTransaction && readConcernArgs.getArgsOpTime()) { + return Status(ErrorCodes::InvalidOptions, + str::stream() + << "The readConcern cannot specify '" + << repl::ReadConcernArgs::kAfterOpTimeFieldName << "' in a transaction"); + } + + // There is no need to check if the command supports the read concern while in a transaction + // because all commands that are allowed to run in a transaction must support all the read + // concerns that can be used with a transaction. + if (!startTransaction && !invocation->supportsReadConcern(readConcernLevel)) { return {ErrorCodes::InvalidOptions, str::stream() << "Command does not support read concern " << readConcernArgs.toString()}; } - // If this command invocation asked for 'majority' read concern, supports blocking majority // reads, and storage engine support for majority reads is disabled, then we set the majority // read mechanism appropriately i.e. we utilize "speculative" read behavior. @@ -820,11 +825,10 @@ void execCommandDatabase(OperationContext* opCtx, // If the parent operation runs in a transaction, we don't override the read concern. auto skipReadConcern = opCtx->getClient()->isInDirectClient() && opCtx->inMultiDocumentTransaction(); + bool startTransaction = static_cast<bool>(sessionOptions.getStartTransaction()); if (!skipReadConcern) { - // If "startTransaction" is present, it must be true due to the parsing above. - const bool upconvertToSnapshot(sessionOptions.getStartTransaction()); auto newReadConcernArgs = uassertStatusOK( - _extractReadConcern(invocation.get(), request.body, upconvertToSnapshot)); + _extractReadConcern(invocation.get(), request.body, startTransaction)); { // We must obtain the client lock to set the ReadConcernArgs on the operation // context as it may be concurrently read by CurrentOp. @@ -833,17 +837,12 @@ void execCommandDatabase(OperationContext* opCtx, } } - if (readConcernArgs.getLevel() == repl::ReadConcernLevel::kSnapshotReadConcern) { - uassert(ErrorCodes::InvalidOptions, - "readConcern level snapshot is only valid for the first transaction operation", - opCtx->getClient()->isInDirectClient() || sessionOptions.getStartTransaction()); - uassert(ErrorCodes::InvalidOptions, - "readConcern level snapshot requires a session ID", - opCtx->getLogicalSessionId()); - uassert(ErrorCodes::InvalidOptions, - "readConcern level snapshot requires a txnNumber", - opCtx->getTxnNumber()); + uassert(ErrorCodes::InvalidOptions, + "read concern level snapshot is only valid in a transaction", + opCtx->inMultiDocumentTransaction() || + readConcernArgs.getLevel() != repl::ReadConcernLevel::kSnapshotReadConcern); + if (startTransaction) { opCtx->lockState()->setSharedLocksShouldTwoPhaseLock(true); opCtx->lockState()->setShouldConflictWithSecondaryBatchApplication(false); } diff --git a/src/mongo/db/stats/server_read_concern_metrics.cpp b/src/mongo/db/stats/server_read_concern_metrics.cpp index 97e77079140..1dd4c9fff8e 100644 --- a/src/mongo/db/stats/server_read_concern_metrics.cpp +++ b/src/mongo/db/stats/server_read_concern_metrics.cpp @@ -51,12 +51,12 @@ ServerReadConcernMetrics* ServerReadConcernMetrics::get(OperationContext* opCtx) } void ServerReadConcernMetrics::recordReadConcern(const repl::ReadConcernArgs& readConcernArgs) { - if (!readConcernArgs.hasOriginalLevel()) { + if (!readConcernArgs.hasLevel()) { _noLevelCount.fetchAndAdd(1); return; } - switch (readConcernArgs.getOriginalLevel()) { + switch (readConcernArgs.getLevel()) { case repl::ReadConcernLevel::kAvailableReadConcern: _levelAvailableCount.fetchAndAdd(1); break; diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp index 6fd7072e82b..e01ffa8a10e 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -586,7 +586,7 @@ void TransactionParticipant::Participant::_setReadSnapshot(OperationContext* opC stdx::lock_guard<Client> lk(*opCtx->getClient()); o(lk).transactionMetricsObserver.onChooseReadTimestamp(readTimestamp); - } else if (readConcernArgs.getOriginalLevel() == repl::ReadConcernLevel::kSnapshotReadConcern) { + } else if (readConcernArgs.getLevel() == repl::ReadConcernLevel::kSnapshotReadConcern) { // For transactions with read concern level specified as 'snapshot', we will use // 'kAllDurableSnapshot' which ensures a snapshot with no 'holes'; that is, it is a state // of the system that could be reconstructed from the oplog. diff --git a/src/mongo/db/transaction_validation.cpp b/src/mongo/db/transaction_validation.cpp index 7e0af2d4a5b..faabf3f766c 100644 --- a/src/mongo/db/transaction_validation.cpp +++ b/src/mongo/db/transaction_validation.cpp @@ -35,6 +35,7 @@ #include "mongo/db/commands.h" #include "mongo/db/commands/txn_cmds_gen.h" +#include "mongo/db/commands/txn_two_phase_commit_cmds_gen.h" #include "mongo/db/logical_session_id.h" #include "mongo/db/write_concern_options.h" @@ -90,6 +91,10 @@ void validateSessionOptions(const OperationSessionInfoFromClient& sessionOptions cmdName != "killCursors"); uassert(ErrorCodes::OperationNotSupportedInTransaction, + "Cannot start a transaction with a prepare", + cmdName != PrepareTransaction::kCommandName); + + uassert(ErrorCodes::OperationNotSupportedInTransaction, "Cannot start a transaction with a commit", cmdName != CommitTransaction::kCommandName); diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 7ef4eb72b96..cc93d78a591 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -403,10 +403,6 @@ void runCommand(OperationContext* opCtx, if (readConcernArgs.getLevel() == repl::ReadConcernLevel::kSnapshotReadConcern) { uassert(ErrorCodes::InvalidOptions, - str::stream() << "read concern snapshot is not supported on mongos for the command " - << commandName, - invocation->supportsReadConcern(readConcernArgs.getLevel())); - uassert(ErrorCodes::InvalidOptions, "read concern snapshot is not supported with atClusterTime on mongos", !readConcernArgs.getArgsAtClusterTime()); } |