SERVER-38690 Mongos should throw write targeting errors during a transaction

buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml

@@ -69,6 +69,7 @@ selector:
   - jstests/sharding/transactions_stale_database_version_errors.js
   - jstests/sharding/transactions_stale_shard_version_errors.js
   - jstests/sharding/transactions_target_at_point_in_time.js
+  - jstests/sharding/transactions_targeting_errors.js
   - jstests/sharding/transactions_view_resolution.js
   - jstests/sharding/transactions_writes_not_retryable.js
   - jstests/sharding/txn_agg.js

buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards_misc.yml

@@ -384,6 +384,7 @@ selector:
   - jstests/sharding/transactions_stale_database_version_errors.js
   - jstests/sharding/transactions_stale_shard_version_errors.js
   - jstests/sharding/transactions_target_at_point_in_time.js
+  - jstests/sharding/transactions_targeting_errors.js
   - jstests/sharding/transactions_view_resolution.js
   - jstests/sharding/transactions_writes_not_retryable.js
   - jstests/sharding/txn_agg.js

jstests/sharding/transactions_targeting_errors.js

@@ -0,0 +1,40 @@
+// Verifies targeting errors encountered in a transaction lead to command response errors.
+//
+// @tags: [uses_transactions]
+(function() {
+    "use strict";
+
+    const dbName = "test";
+    const collName = "foo";
+    const ns = dbName + "." + collName;
+
+    const st = new ShardingTest({shards: 2});
+    assert.commandWorked(st.s.adminCommand({enableSharding: dbName}));
+    assert.commandWorked(st.s.adminCommand({shardCollection: ns, key: {skey: "hashed"}}));
+
+    const session = st.s.startSession();
+    const sessionDB = session.getDatabase("test");
+
+    // Failed update.
+
+    session.startTransaction();
+
+    let res = sessionDB.runCommand(
+        {update: collName, updates: [{q: {skey: {$lte: 5}}, u: {$set: {x: 1}}, multi: false}]});
+    assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions);
+    assert(!res.hasOwnProperty("writeErrors"), "expected no write errors, res: " + tojson(res));
+
+    session.abortTransaction_forTesting();
+
+    // Failed delete.
+
+    session.startTransaction();
+
+    res = sessionDB.runCommand({delete: collName, deletes: [{q: {skey: {$lte: 5}}, limit: 1}]});
+    assert.commandFailedWithCode(res, ErrorCodes.ShardKeyNotFound);
+    assert(!res.hasOwnProperty("writeErrors"), "expected no write errors, res: " + tojson(res));
+
+    session.abortTransaction_forTesting();
+
+    st.stop();
+}());

src/mongo/s/write_ops/batch_write_op.cpp

@@ -36,6 +36,7 @@
 
 #include "mongo/base/error_codes.h"
 #include "mongo/db/operation_context.h"
+#include "mongo/s/transaction_router.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/util/transitional_tools_do_not_use/vector_spooling.h"
 
@@ -318,6 +319,13 @@ Status BatchWriteOp::targetBatch(const NSTargeter& targeter,
         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);
 

src/mongo/s/write_ops/batch_write_op_test.cpp

@@ -32,6 +32,9 @@
 
 #include "mongo/base/owned_pointer_map.h"
 #include "mongo/db/operation_context_noop.h"
+#include "mongo/s/session_catalog_router.h"
+#include "mongo/s/sharding_router_test_fixture.h"
+#include "mongo/s/transaction_router.h"
 #include "mongo/s/write_ops/batch_write_op.h"
 #include "mongo/s/write_ops/batched_command_request.h"
 #include "mongo/s/write_ops/mock_ns_targeter.h"
@@ -1545,5 +1548,91 @@ TEST_F(BatchWriteOpLimitTests, OneBigOneSmall) {
     ASSERT(batchOp.isFinished());
 }
 
+class BatchWriteOpTransactionTest : public ShardingTestFixture {
+public:
+    const TxnNumber kTxnNumber = 5;
+
+    void setUp() override {
+        ShardingTestFixture::setUp();
+
+        operationContext()->setLogicalSessionId(makeLogicalSessionIdForTest());
+        operationContext()->setTxnNumber(kTxnNumber);
+        repl::ReadConcernArgs::get(operationContext()) =
+            repl::ReadConcernArgs(repl::ReadConcernLevel::kSnapshotReadConcern);
+
+        _scopedSession.emplace(operationContext());
+
+        auto txnRouter = TransactionRouter::get(operationContext());
+        txnRouter->beginOrContinueTxn(
+            operationContext(), kTxnNumber, TransactionRouter::TransactionActions::kStart);
+    }
+
+    void tearDown() override {
+        _scopedSession.reset();
+        repl::ReadConcernArgs::get(operationContext()) = repl::ReadConcernArgs();
+
+        ShardingTestFixture::tearDown();
+    }
+
+private:
+    boost::optional<RouterOperationContextSession> _scopedSession;
+};
+
+TEST_F(BatchWriteOpTransactionTest, ThrowTargetingErrorsInTransaction_Delete) {
+    NamespaceString nss("foo.bar");
+    ShardEndpoint endpoint(ShardId("shard"), ChunkVersion::IGNORED());
+    MockNSTargeter targeter;
+    initTargeterHalfRange(nss, endpoint, &targeter);
+
+    // Untargetable delete op.
+    BatchedCommandRequest deleteRequest([&] {
+        write_ops::Delete deleteOp(nss);
+        deleteOp.setDeletes({buildDelete(BSON("x" << 1), false)});
+        return deleteOp;
+    }());
+    BatchWriteOp batchOp(operationContext(), deleteRequest);
+
+    OwnedPointerMap<ShardId, TargetedWriteBatch> targetedOwned;
+    std::map<ShardId, TargetedWriteBatch*>& targeted = targetedOwned.mutableMap();
+
+    bool recordTargetErrors = false;
+    ASSERT_THROWS_CODE(batchOp.targetBatch(targeter, recordTargetErrors, &targeted),
+                       AssertionException,
+                       ErrorCodes::UnknownError);
+
+    recordTargetErrors = true;
+    ASSERT_THROWS_CODE(batchOp.targetBatch(targeter, recordTargetErrors, &targeted),
+                       AssertionException,
+                       ErrorCodes::UnknownError);
+}
+
+TEST_F(BatchWriteOpTransactionTest, ThrowTargetingErrorsInTransaction_Update) {
+    NamespaceString nss("foo.bar");
+    ShardEndpoint endpoint(ShardId("shard"), ChunkVersion::IGNORED());
+    MockNSTargeter targeter;
+    initTargeterHalfRange(nss, endpoint, &targeter);
+
+    // Untargetable update op.
+    BatchedCommandRequest updateRequest([&] {
+        write_ops::Update updateOp(nss);
+        updateOp.setUpdates({buildUpdate(BSON("x" << 1), BSONObj(), false)});
+        return updateOp;
+    }());
+    BatchWriteOp batchOp(operationContext(), updateRequest);
+
+    OwnedPointerMap<ShardId, TargetedWriteBatch> targetedOwned;
+    std::map<ShardId, TargetedWriteBatch*>& targeted = targetedOwned.mutableMap();
+
+    bool recordTargetErrors = false;
+    ASSERT_THROWS_CODE(batchOp.targetBatch(targeter, recordTargetErrors, &targeted),
+                       AssertionException,
+                       ErrorCodes::UnknownError);
+
+    recordTargetErrors = true;
+    ASSERT_THROWS_CODE(batchOp.targetBatch(targeter, recordTargetErrors, &targeted),
+                       AssertionException,
+                       ErrorCodes::UnknownError);
+}
+
 }  // namespace
 }  // namespace mongo