SERVER-87191 Respect snapshot read concern for writes without shard key (#20058)r7.3.1-rc1

(cherry picked from commit 45483ea25e7370f3a5f0d33e4b41b0f8fd026fcd)

GitOrigin-RevId: 0c82e6ac11b46062aab07868d7cc3366469cb731

5 files changed, 91 insertions, 9 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 936e92585e8..5620e140f7e 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -569,6 +569,8 @@ last-continuous:
     ticket: SERVER-85848
   - test_file: jstests/sharding/timeseries_drop.js
     ticket: SERVER-84589
+  - test_file: jstests/sharding/updateOne_without_shard_key/write_without_shard_key_single_shard_data_placement_change.js
+    ticket: SERVER-87191
   suites: null
 last-lts:
   all:
diff --git a/jstests/noPassthrough/bulk_write_metrics.js b/jstests/noPassthrough/bulk_write_metrics.js
index 25973e158f2..d4ccfaf91ba 100644
--- a/jstests/noPassthrough/bulk_write_metrics.js
+++ b/jstests/noPassthrough/bulk_write_metrics.js
@@ -228,7 +228,9 @@ function runTest(isMongos, cluster, bulkWrite, retryCount, timeseries) {
 
     const retryCount = 3;
     for (const bulkWrite of [false, true]) {
-        for (const timeseries of [false, true]) {
+        // TODO: SERVER-88153 Check metrics for timeseries writes. This test may need to be adjusted
+        // since it's possible a timeseries write may only target a single shard.
+        for (const timeseries of [false]) {
             runTest(true /* isMongos */, st, bulkWrite, retryCount, timeseries);
         }
     }
diff --git a/jstests/sharding/updateOne_without_shard_key/write_without_shard_key_single_shard_data_placement_change.js b/jstests/sharding/updateOne_without_shard_key/write_without_shard_key_single_shard_data_placement_change.js
new file mode 100644
index 00000000000..6681d308ea9
--- /dev/null
+++ b/jstests/sharding/updateOne_without_shard_key/write_without_shard_key_single_shard_data_placement_change.js
@@ -0,0 +1,85 @@
+/**
+ * Test writes without shard key in a transaction with snapshot read concern fails during data
+ * placement change.
+ *
+ * @tags: [
+ *    requires_fcv_71,
+ *    requires_sharding,
+ *    uses_transactions,
+ * ]
+ */
+
+import {
+    WriteWithoutShardKeyTestUtil
+} from "jstests/sharding/updateOne_without_shard_key/libs/write_without_shard_key_test_util.js";
+// 2 shards single node, 1 mongos, 1 config server 3-node.
+const st = new ShardingTest({});
+const dbName = "testDb";
+const collName = "testColl";
+const collName2 = "testColl2";
+const nss = dbName + "." + collName;
+const splitPoint = 0;
+const docsToInsert = [
+    {_id: 1, x: -1, y: 1},
+    {_id: 2, x: 1, y: 2},
+];
+const dbConn = st.s.getDB(dbName);
+const coll = dbConn.getCollection(collName);
+
+// Sets up a 2 shard cluster using 'x' as a shard key where Shard 0 owns x <
+// splitPoint and Shard 1 splitPoint >= 0.
+WriteWithoutShardKeyTestUtil.setupShardedCollection(
+    st, nss, {x: 1}, [{x: splitPoint}], [{query: {x: splitPoint}, shard: st.shard1.shardName}]);
+
+assert.commandWorked(coll.insert(docsToInsert));
+
+function runTest(testCase) {
+    const session = st.s.startSession();
+    session.startTransaction({readConcern: {level: "snapshot"}});
+    session.getDatabase(dbName).getCollection(collName2).insert({x: 1});
+
+    // Move all chunks for testDb.testColl to shard0.
+    assert.commandWorked(
+        st.s.adminCommand({moveChunk: nss, find: {x: 0}, to: st.shard0.shardName}));
+
+    // This find and modify MUST fail, the data moved to another shard, we can't try on shard0 nor
+    // shard1 with the original clusterTime of the transaction.
+    assert.commandFailedWithCode(session.getDatabase(dbName).runCommand(testCase.cmdObj),
+                                 ErrorCodes.MigrationConflict);
+
+    // Reset the chunk distribution for the next test.
+    assert.commandWorked(
+        st.s.adminCommand({moveChunk: nss, find: {x: 0}, to: st.shard1.shardName}));
+}
+
+let testCases = [
+    {
+        logMessage: "Running updateOne test",
+        cmdObj: {
+            update: collName,
+            updates: [{q: {y: 2}, u: {$inc: {z: 1}}}],
+        },
+    },
+    {
+        logMessage: "Running findAndModify test",
+        cmdObj: {
+            findAndModify: collName,
+            query: {y: 2},
+            update: {$inc: {z: 1}},
+        }
+    },
+    {
+        logMessage: "Running deleteOne test.",
+        cmdObj: {
+            delete: collName,
+            deletes: [{q: {y: 2}, limit: 1}],
+        },
+    }
+];
+
+testCases.forEach(testCase => {
+    jsTestLog(testCase.logMessage);
+    runTest(testCase);
+});
+
+st.stop();
diff --git a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
index fa7d50d7c65..692ea32a96c 100644
--- a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
@@ -543,13 +543,6 @@ boost::optional<ShardId> targetPotentiallySingleShard(
     const BSONObj& query,
     const BSONObj& collation,
     bool isTimeseriesViewRequest) {
-    // Special case: there's only one shard owning all the chunks.
-    if (cm.getNShardsOwningChunks() == 1) {
-        std::set<ShardId> shardIds;
-        cm.getAllShardIds(&shardIds);
-        return *shardIds.begin();
-    }
-
     std::set<ShardId> shardIds;
     getShardIdsForQuery(expCtx,
                         getQueryForShardKey(expCtx, cm, query, isTimeseriesViewRequest),
diff --git a/src/mongo/s/write_ops/batch_write_exec.cpp b/src/mongo/s/write_ops/batch_write_exec.cpp
index a5b32010977..33cd4dc067e 100644
--- a/src/mongo/s/write_ops/batch_write_exec.cpp
+++ b/src/mongo/s/write_ops/batch_write_exec.cpp
@@ -737,7 +737,7 @@ void executeNonOrdinaryWriteChildBatches(OperationContext* opCtx,
 
             // If there is only 1 targetable shard, we can skip using the two phase write
             // protocol.
-            if (targeter.getNShardsOwningChunks() == 1) {
+            if (childBatches.size() == 1) {
                 executeChildBatches(opCtx,
                                     targeter,
                                     clientRequest,