SERVER-84089 Check for collation when retrying bulk write upsert (#29…

…487)

GitOrigin-RevId: 80db4dc55be1361b119f257e3d133392fecee09c

etc/backports_required_for_multiversion_tests.yml

@@ -561,6 +561,8 @@ last-continuous:
     ticket: SERVER-92904
   - test_file: jstests/core/ddl/create_indexes.js
     ticket: SERVER-90952
+  - test_file: jstests/core/write/update/upsert_duplicate_key_retry_collation.js
+    ticket: SERVER-84089
   - test_file: jstests/core/timeseries/timeseries_filter_extended_range.js
     ticket: SERVER-94207
   suites: null
@@ -1176,6 +1178,8 @@ last-lts:
     ticket: SERVER-92904
   - test_file: jstests/core/ddl/create_indexes.js
     ticket: SERVER-90952
+  - test_file: jstests/core/write/update/upsert_duplicate_key_retry_collation.js
+    ticket: SERVER-84089
   - test_file: jstests/core/timeseries/timeseries_filter_extended_range.js
     ticket: SERVER-94207
   suites: null

jstests/core/write/update/upsert_duplicate_key_retry_collation.js

@@ -0,0 +1,52 @@
+/**
+ * When two concurrent identical upsert operations are performed, for which a unique index exists on
+ * the query values, it is possible that they will both attempt to perform an insert with one of
+ * the two failing on the unique index constraint. This test confirms that we respect collation
+ * when deciding to retry the error.
+ *
+ * @tags: [assumes_unsharded_collection]
+ */
+
+const testColl = db.upsert_duplicate_key_retry_collation;
+
+{  // Test index without collation, query with collation
+    testColl.drop();
+    assert.commandWorked(testColl.createIndex({x: 1}, {unique: true}));
+    assert.commandWorked(testColl.insertMany([{_id: 0, x: "UNIQUE"}, {_id: 1, x: "unique"}]));
+
+    assert.throwsWithCode(() => {
+        testColl.updateOne({x: "unique"},
+                           {$set: {x: "unique"}},
+                           {upsert: true, collation: {locale: "en", strength: 1}});
+        testColl.updateOne({x: "unique"},
+                           {$set: {x: "UNIQUE"}},
+                           {upsert: true, collation: {locale: "en", strength: 1}});
+    }, ErrorCodes.DuplicateKey);
+}
+
+{  // Test index with collation, query without collation
+    testColl.drop();
+    assert.commandWorked(
+        testColl.createIndex({x: 1}, {unique: true, collation: {locale: "en", strength: 2}}));
+    assert.commandWorked(testColl.insertMany([{_id: 0, x: "UNIQUE"}]));
+
+    assert.throwsWithCode(
+        () => testColl.updateOne({x: "unique"}, {$set: {x: "Unique"}}, {upsert: true}),
+        ErrorCodes.DuplicateKey);
+}
+
+{  // Test index and query with different collation strength
+    testColl.drop();
+    assert.commandWorked(
+        testColl.createIndex({x: 1}, {unique: true, collation: {locale: "en", strength: 3}}));
+    assert.commandWorked(testColl.insertMany([{_id: 0, x: "UNIQUË"}, {_id: 1, x: "UNIQUE"}]));
+
+    assert.throwsWithCode(() => {
+        testColl.updateOne({x: "UNIQUE"},
+                           {$set: {x: "UNIQUE"}},
+                           {upsert: true, collation: {locale: "en", strength: 1}});
+        testColl.updateOne({x: "UNIQUE"},
+                           {$set: {x: "UNIQUË"}},
+                           {upsert: true, collation: {locale: "en", strength: 1}});
+    }, ErrorCodes.DuplicateKey);
+}

jstests/noPassthrough/upsert_duplicate_key_retry.js

@@ -10,7 +10,8 @@
 (function() {
 "use strict";
 
-load("jstests/libs/curop_helpers.js");  // For waitForCurOpByFailPoint().
+load("jstests/libs/curop_helpers.js");    // For waitForCurOpByFailPoint().
+load("jstests/libs/fail_point_util.js");  // For configureFailPoint().
 
 const rst = new ReplSetTest({nodes: 1});
 rst.startSet();
@@ -33,20 +34,13 @@ function performUpsert() {
 assert.commandWorked(testColl.createIndex({x: 1}, {unique: true}));
 
 // Will hang upsert operations just prior to performing an insert.
-assert.commandWorked(
-    testDB.adminCommand({configureFailPoint: "hangBeforeUpsertPerformsInsert", mode: "alwaysOn"}));
-
+const failPoint = configureFailPoint(testDB, "hangBeforeUpsertPerformsInsert");
 const awaitUpdate1 = startParallelShell(performUpsert, rst.ports[0]);
 const awaitUpdate2 = startParallelShell(performUpsert, rst.ports[0]);
 
 // Query current operations until 2 matching operations are found.
-assert.soon(() => {
-    const curOps = waitForCurOpByFailPointNoNS(adminDB, "hangBeforeUpsertPerformsInsert");
-    return curOps.length === 2;
-});
-
-assert.commandWorked(
-    testDB.adminCommand({configureFailPoint: "hangBeforeUpsertPerformsInsert", mode: "off"}));
+failPoint.wait({timesEntered: 2});
+failPoint.off();
 
 awaitUpdate1();
 awaitUpdate2();

src/mongo/db/ops/write_ops_exec.cpp

@@ -1927,6 +1927,24 @@ bool shouldRetryDuplicateKeyException(const ParsedUpdate& parsedUpdate,
         return false;
     }
 
+    // Check that collation of the query matches the unique index. To avoid calling
+    // CollatorFactoryInterface when possible, first check the simple collator case.
+    const auto& canonicalQuery = *parsedUpdate.getParsedQuery();
+    bool queryHasSimpleCollator = canonicalQuery.getCollator() == nullptr;
+    bool indexHasSimpleCollator = errorInfo.getCollation().isEmpty();
+    if (queryHasSimpleCollator != indexHasSimpleCollator) {
+        return false;
+    }
+
+    if (!indexHasSimpleCollator) {
+        auto indexCollator = uassertStatusOK(
+            CollatorFactoryInterface::get(canonicalQuery.getOpCtx()->getServiceContext())
+                ->makeFromBSON(errorInfo.getCollation()));
+        if (!CollatorInterface::collatorsMatch(canonicalQuery.getCollator(), indexCollator.get())) {
+            return false;
+        }
+    }
+
     auto keyValue = errorInfo.getDuplicatedKeyValue();
 
     BSONObjIterator keyPatternIter(keyPattern);

src/mongo/db/storage/duplicate_key_error_info.h

@@ -81,6 +81,10 @@ class DuplicateKeyErrorInfo final : public ErrorExtraInfo {
         return _duplicateRid;
     }
 
+    const BSONObj& getCollation() const {
+        return _collation;
+    }
+
 private:
     BSONObj _keyPattern;
     BSONObj _keyValue;