diff options
author | Jack Mulrow <jack.mulrow@mongodb.com> | 2022-05-11 21:56:30 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-05-28 02:17:24 +0000 |
commit | b054e4fb3c43c15f55d7ae45f5b5e3957dd2db15 (patch) | |
tree | 26bcbe60c36beb5d6e516335276ab1120f285a8d | |
parent | 233249600339aed492927b8d252f856366a396dc (diff) | |
download | mongo-b054e4fb3c43c15f55d7ae45f5b5e3957dd2db15.tar.gz |
SERVER-66388 Disallow using transaction API in operations with shard versions
(cherry picked from commit 51bff7a1d0145afebb57a04e82bd962985801f06)
-rw-r--r-- | src/mongo/db/SConscript | 2 | ||||
-rw-r--r-- | src/mongo/db/cluster_transaction_api.h | 6 | ||||
-rw-r--r-- | src/mongo/db/transaction_api.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/transaction_api.h | 23 | ||||
-rw-r--r-- | src/mongo/db/transaction_api_test.cpp | 55 |
5 files changed, 97 insertions, 1 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 8e5de43635d..672c234dd3d 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -858,6 +858,7 @@ env.Library( '$BUILD_DIR/mongo/db/auth/auth', '$BUILD_DIR/mongo/db/query/command_request_response', '$BUILD_DIR/mongo/db/query/query_request', + '$BUILD_DIR/mongo/db/s/sharding_api_d', '$BUILD_DIR/mongo/executor/task_executor_interface', '$BUILD_DIR/mongo/rpc/command_status', '$BUILD_DIR/mongo/rpc/rpc', @@ -2723,6 +2724,7 @@ if wiredtiger: '$BUILD_DIR/mongo/db/mongohasher', '$BUILD_DIR/mongo/db/query/common_query_enums_and_helpers', '$BUILD_DIR/mongo/db/query/query_test_service_context', + '$BUILD_DIR/mongo/db/s/sharding_api_d', '$BUILD_DIR/mongo/db/storage/wiredtiger/storage_wiredtiger', '$BUILD_DIR/mongo/executor/async_timer_mock', '$BUILD_DIR/mongo/idl/idl_parser', diff --git a/src/mongo/db/cluster_transaction_api.h b/src/mongo/db/cluster_transaction_api.h index 45135cde65d..254b38791bc 100644 --- a/src/mongo/db/cluster_transaction_api.h +++ b/src/mongo/db/cluster_transaction_api.h @@ -46,6 +46,12 @@ public: Future<DbResponse> handleRequest(OperationContext* opCtx, const Message& request) const override; + + bool canRunInShardedOperations() const { + // Cluster commands will attach appropriate shard versions for any targeted namespaces, so + // it is safe to use this client within a caller's operation with shard versions. + return true; + } }; } // namespace mongo::txn_api::details diff --git a/src/mongo/db/transaction_api.cpp b/src/mongo/db/transaction_api.cpp index 9208c7936a5..a2d22205c03 100644 --- a/src/mongo/db/transaction_api.cpp +++ b/src/mongo/db/transaction_api.cpp @@ -49,6 +49,7 @@ #include "mongo/db/query/getmore_command_gen.h" #include "mongo/db/repl/read_concern_args.h" #include "mongo/db/repl/repl_client_info.h" +#include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/session_catalog.h" #include "mongo/db/transaction_validation.h" #include "mongo/db/write_concern_options.h" @@ -672,6 +673,17 @@ void Transaction::_setSessionInfo(WithLock, } void Transaction::_primeTransaction(OperationContext* opCtx) { + // The API does not forward shard or database versions from the caller's opCtx, so spawned + // commands would not obey sharding protocols, like the migration critical section, so it + // cannot currently be used in an operation with shard versions. This does not apply in the + // cluster commands configuration because those commands will attach appropriate shard + // versions. + uassert(6638800, + "Transaction API does not currently support use within operations with shard or " + "database versions without using router commands", + !OperationShardingState::isComingFromRouter(opCtx) || + _txnClient->canRunInShardedOperations()); + stdx::lock_guard<Latch> lg(_mutex); // Extract session options and infer execution context from client's opCtx. diff --git a/src/mongo/db/transaction_api.h b/src/mongo/db/transaction_api.h index c224949863a..9f7ca9e7abb 100644 --- a/src/mongo/db/transaction_api.h +++ b/src/mongo/db/transaction_api.h @@ -136,6 +136,11 @@ public: * implementations. */ virtual bool supportsClientTransactionContext() const = 0; + + /** + * Returns if the client is safe to use within an operation with a shard or database version. + */ + virtual bool canRunInShardedOperations() const = 0; }; using Callback = @@ -158,7 +163,6 @@ public: * * Optionally accepts a custom TransactionClient and will default to a client that runs commands * against the local service entry point. - * */ SyncTransactionWithRetries(OperationContext* opCtx, std::shared_ptr<executor::TaskExecutor> executor, @@ -220,6 +224,12 @@ public: */ virtual Future<DbResponse> handleRequest(OperationContext* opCtx, const Message& request) const = 0; + + /** + * Returns if a client with these behaviors is safe to use within an operation with a shard or + * database version. + */ + virtual bool canRunInShardedOperations() const = 0; }; /** @@ -234,6 +244,13 @@ public: Future<DbResponse> handleRequest(OperationContext* opCtx, const Message& request) const override; + + bool canRunInShardedOperations() const { + // Commands are run directly on the local service entry point, so if the caller is in an + // operation that requires shard versions, spawned commands won't include shard versions and + // won't obey sharding protocols. + return false; + } }; /** @@ -272,6 +289,10 @@ public: return true; } + virtual bool canRunInShardedOperations() const override { + return _behaviors->canRunInShardedOperations(); + } + private: ServiceContext* const _serviceContext; std::shared_ptr<executor::TaskExecutor> _executor; diff --git a/src/mongo/db/transaction_api_test.cpp b/src/mongo/db/transaction_api_test.cpp index a417a4d3028..a6cb59babab 100644 --- a/src/mongo/db/transaction_api_test.cpp +++ b/src/mongo/db/transaction_api_test.cpp @@ -36,6 +36,7 @@ #include "mongo/db/error_labels.h" #include "mongo/db/logical_session_id_helpers.h" #include "mongo/db/operation_context.h" +#include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/service_context.h" #include "mongo/db/service_context_test_fixture.h" #include "mongo/db/transaction_api.h" @@ -180,6 +181,10 @@ public: return true; } + virtual bool canRunInShardedOperations() const override { + return false; + } + BSONObj getLastSentRequest() { if (_sentRequests.empty()) { return BSONObj(); @@ -374,6 +379,13 @@ protected: opCtx(), _executor, std::move(resourceYielder), std::move(mockClient)); } + void resetTxnWithRetriesWithClient(std::unique_ptr<txn_api::TransactionClient> txnClient) { + waitForAllEarlierTasksToComplete(); + _txnWithRetries = nullptr; + _txnWithRetries = std::make_unique<txn_api::SyncTransactionWithRetries>( + opCtx(), _executor, nullptr, std::move(txnClient)); + } + void expectSentAbort(TxnNumber txnNumber, BSONObj writeConcern) { auto lastRequest = mockClient()->getLastSentRequest(); assertTxnMetadata(lastRequest, @@ -1888,5 +1900,48 @@ TEST_F(TxnAPITest, MaxTimeMSIsSetIfOperationContextHasDeadlineAndIgnoresDefaultR assertSessionIdMetadata(mockClient()->getLastSentRequest(), LsidAssertion::kStandalone); ASSERT_EQ(lastRequest.firstElementFieldNameStringData(), "commitTransaction"_sd); } + +TEST_F(TxnAPITest, CannotBeUsedWithinShardedOperationsIfClientDoesNotSupportIt) { + OperationShardingState::setShardRole( + opCtx(), NamespaceString("foo.bar"), ChunkVersion(), boost::none); + + ASSERT_THROWS_CODE( + resetTxnWithRetries(), DBException, ErrorCodes::duplicateCodeForTest(6638800)); +} + +class MockShardedOperationTransactionClient : public txn_api::TransactionClient { +public: + virtual void injectHooks(std::unique_ptr<txn_api::details::TxnMetadataHooks> hooks) {} + + virtual SemiFuture<BSONObj> runCommand(StringData dbName, BSONObj cmd) const { + MONGO_UNREACHABLE; + } + + virtual SemiFuture<BatchedCommandResponse> runCRUDOp(const BatchedCommandRequest& cmd, + std::vector<StmtId> stmtIds) const { + MONGO_UNREACHABLE; + } + + virtual SemiFuture<std::vector<BSONObj>> exhaustiveFind(const FindCommandRequest& cmd) const { + MONGO_UNREACHABLE; + } + + virtual bool supportsClientTransactionContext() const { + MONGO_UNREACHABLE; + } + + virtual bool canRunInShardedOperations() const override { + return true; + } +}; + +TEST_F(TxnAPITest, CanBeUsedWithinShardedOperationsIfClientSupportsIt) { + OperationShardingState::setShardRole( + opCtx(), NamespaceString("foo.bar"), ChunkVersion(), boost::none); + + // Should not throw. + resetTxnWithRetriesWithClient(std::make_unique<MockShardedOperationTransactionClient>()); +} + } // namespace } // namespace mongo |