commit bd48d809adaa3d965f7c9c9246a56c962751ea06 Merge: 9a7cfb73da 47eef8ca29 Author: A. Jesse Jiryu Davis Date: Mon Mar 11 11:33:40 2019 -0400 WIP on SERVER-39175-mongos-writeerrors-in-txns diff --cc buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml index 5f7ea4cea9,5f7ea4cea9..778924241f --- a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml @@@ -40,9 -40,9 +40,6 @@@ selector # Does not use the transactions shell helpers so afterClusterTime read concern is incorrectly # attached to statements in a transaction beyond the first one. - jstests/core/txns/non_transactional_operations_on_session_with_transaction.js -- -- # TODO SERVER-39175: Mongos doesn't correctly report write errors in transactions -- - jstests/core/txns/multi_statement_transaction_write_error.js exclude_with_any_tags: # Prepare is not a command on mongos. - uses_prepare_transaction diff --cc buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml index e220d8e178,e220d8e178..5208fca1a4 --- a/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml @@@ -42,9 -42,9 +42,6 @@@ selector # Does not use the transactions shell helpers so afterClusterTime read concern is incorrectly # attached to statements in a transaction beyond the first one. - jstests/core/txns/non_transactional_operations_on_session_with_transaction.js -- -- # TODO SERVER-39175: Mongos doesn't correctly report write errors in transactions -- - jstests/core/txns/multi_statement_transaction_write_error.js exclude_with_any_tags: - assumes_against_mongod_not_mongos # Tests tagged with the following will fail because they assume collections are not sharded. diff --cc buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml index 76693bb47b,76693bb47b..ec173213a1 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml @@@ -36,9 -36,9 +36,6 @@@ selector # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js -- -- # TODO SERVER-39175: Mongos doesn't correctly report write errors in transactions -- - jstests/core/txns/multi_statement_transaction_write_error.js exclude_with_any_tags: # Prepare is not a command on mongos. - uses_prepare_transaction diff --cc buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml index f018c8df30,f018c8df30..0753da6bf8 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml @@@ -38,9 -38,9 +38,6 @@@ selector # View tests aren't expected to work when collections are implicitly sharded. - jstests/core/txns/view_reads_in_transaction.js -- -- # TODO SERVER-39175: Mongos doesn't correctly report write errors in transactions -- - jstests/core/txns/multi_statement_transaction_write_error.js exclude_with_any_tags: - assumes_against_mongod_not_mongos # Tests tagged with the following will fail because they assume collections are not sharded. diff --cc jstests/core/txns/multi_statement_transaction_write_error.js index a825c9d997,a825c9d997..f710cf5953 --- a/jstests/core/txns/multi_statement_transaction_write_error.js +++ b/jstests/core/txns/multi_statement_transaction_write_error.js @@@ -24,6 -24,6 +24,7 @@@ // Assert that "cmd" fails with error "code" after "nExpected" operations, or fail with "msg" function runInTxn({cmd, msg, code, nExpected}) { ++ jsTestLog(msg); const session = db.getMongo().startSession(); session.startTransaction(); try { diff --cc src/mongo/s/write_ops/batch_write_exec.cpp index d2a0a7a05e,d2a0a7a05e..e5cf4f1ebe --- a/src/mongo/s/write_ops/batch_write_exec.cpp +++ b/src/mongo/s/write_ops/batch_write_exec.cpp @@@ -149,8 -149,8 +149,9 @@@ void BatchWriteExec::executeBatch(Opera const size_t numToSend = childBatches.size(); size_t numSent = 0; ++ auto batchFailed = false; -- while (numSent != numToSend) { ++ while (numSent != numToSend && !batchFailed) { // Collect batches out on the network, mapped by endpoint OwnedShardBatchMap ownedPendingBatches; OwnedShardBatchMap::MapType& pendingBatches = ownedPendingBatches.mutableMap(); @@@ -281,10 -281,10 +282,8 @@@ // Note: this returns a bad status if any part of the batch failed. auto batchStatus = batchedCommandResponse.toStatus(); if (!batchStatus.isOK()) { -- batchOp.forgetTargetedBatchesOnTransactionAbortingError(); -- uassertStatusOK(batchStatus.withContext( -- str::stream() << "Encountered error from " << shardHost.toString() -- << " during a transaction")); ++ batchFailed = true; ++ break; } } @@@ -335,10 -335,10 +334,8 @@@ // If we are in a transaction, we must fail the whole batch on any error. if (TransactionRouter::get(opCtx)) { -- batchOp.forgetTargetedBatchesOnTransactionAbortingError(); -- uassertStatusOK(status.withContext( -- str::stream() << "Encountered error from " << shardHost.toString() -- << " during a transaction")); ++ batchFailed = true; ++ break; } } } diff --cc src/mongo/s/write_ops/batch_write_op.cpp index 65f6926215,65f6926215..44597de1f3 --- a/src/mongo/s/write_ops/batch_write_op.cpp +++ b/src/mongo/s/write_ops/batch_write_op.cpp @@@ -318,17 -318,17 +318,10 @@@ Status BatchWriteOp::targetBatch(const Status targetStatus = writeOp.targetWrites(_opCtx, targeter, &writes); if (!targetStatus.isOK()) { -- // Throw any error encountered during a transaction, since the whole batch must fail. -- if (TransactionRouter::get(_opCtx)) { -- forgetTargetedBatchesOnTransactionAbortingError(); -- uassertStatusOK(targetStatus.withContext( -- str::stream() << "Encountered targeting error during a transaction")); -- } -- WriteErrorDetail targetError; buildTargetError(targetStatus, &targetError); -- if (!recordTargetErrors) { ++ if (!recordTargetErrors || TransactionRouter::get(_opCtx)) { // Cancel current batch state with an error _cancelBatches(targetError, std::move(batchMap)); return targetStatus; @@@ -688,10 -688,10 +681,6 @@@ void BatchWriteOp::abortBatch(const Wri dassert(isFinished()); } --void BatchWriteOp::forgetTargetedBatchesOnTransactionAbortingError() { -- _targeted.clear(); --} -- bool BatchWriteOp::isFinished() { const size_t numWriteOps = _clientRequest.sizeWriteOps(); const bool orderedOps = _clientRequest.getWriteCommandBase().getOrdered(); diff --cc src/mongo/s/write_ops/batch_write_op.h index a0ce02117d,a0ce02117d..9401e9e388 --- a/src/mongo/s/write_ops/batch_write_op.h +++ b/src/mongo/s/write_ops/batch_write_op.h @@@ -173,13 -173,13 +173,6 @@@ public */ void abortBatch(const WriteErrorDetail& error); -- /** -- * Disposes of all tracked targeted batches when an error is encountered during a transaction. -- * This is safe because any partially written data on shards will be rolled back if mongos -- * decides to abort. -- */ -- void forgetTargetedBatchesOnTransactionAbortingError(); -- /** * Returns false if the batch write op needs more processing. */