SERVER-61427 Skip duplicate index key handler when exact key already exists in index

(cherry picked from commit 9f15b2b1eeb356f4e3d0c6e06302941ec685cc0a)
(cherry picked from commit cbfe08edee4a6aa41694b73be690b4e56de0bb3a)

7 files changed, 96 insertions, 76 deletions

diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index 8341b81c73d..1236d1c85e0 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -198,19 +198,28 @@ Status AbstractIndexAccessMethod::insertKeys(OperationContext* opCtx,
     // Add all new keys into the index. The RecordId for each is already encoded in the KeyString.
     for (const auto& keyString : keys) {
         bool unique = _descriptor->unique();
-        Status status = _newInterface->insert(opCtx, keyString, !unique /* dupsAllowed */);
+        auto result = _newInterface->insert(opCtx, keyString, !unique /* dupsAllowed */);
 
         // When duplicates are encountered and allowed, retry with dupsAllowed. Call
         // onDuplicateKey() with the inserted duplicate key.
-        if (ErrorCodes::DuplicateKey == status.code() && options.dupsAllowed) {
+        if (ErrorCodes::DuplicateKey == result.getStatus().code() && options.dupsAllowed) {
             invariant(unique);
-            status = _newInterface->insert(opCtx, keyString, true /* dupsAllowed */);
 
-            if (status.isOK() && onDuplicateKey)
-                status = onDuplicateKey(keyString);
+            result = _newInterface->insert(opCtx, keyString, true /* dupsAllowed */);
+            if (!result.isOK()) {
+                return result.getStatus();
+            } else if (result.getValue() && onDuplicateKey) {
+                // Only run the duplicate key handler if we inserted the key ourselves. Someone else
+                // could have already inserted this exact key, but in that case we don't count it as
+                // a duplicate.
+                auto status = onDuplicateKey(keyString);
+                if (!status.isOK()) {
+                    return status;
+                }
+            }
+        } else if (!result.isOK()) {
+            return result.getStatus();
         }
-        if (!status.isOK())
-            return status;
     }
     if (numInserted) {
         *numInserted = keys.size();
@@ -455,9 +464,9 @@ Status AbstractIndexAccessMethod::update(OperationContext* opCtx,
 
     // Add all new data keys into the index.
     for (const auto& keyString : ticket.added) {
-        Status status = _newInterface->insert(opCtx, keyString, ticket.dupsAllowed);
-        if (!status.isOK())
-            return status;
+        auto result = _newInterface->insert(opCtx, keyString, ticket.dupsAllowed);
+        if (!result.isOK())
+            return result.getStatus();
     }
 
     // If these keys should cause the index to become multikey, pass them into the catalog.
diff --git a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
index f56087b7a95..0855f2842d0 100644
--- a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
+++ b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
@@ -186,10 +186,10 @@ public:
         return {};
     }
 
-    virtual Status insert(OperationContext* opCtx,
-                          const KeyString::Value& keyString,
-                          bool dupsAllowed) {
-        return Status::OK();
+    virtual StatusWith<bool> insert(OperationContext* opCtx,
+                                    const KeyString::Value& keyString,
+                                    bool dupsAllowed) {
+        return true;
     }
 
     virtual void unindex(OperationContext* opCtx,
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.cpp
index c8a3c12aec1..52be28c2183 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.cpp
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.cpp
@@ -1359,9 +1359,9 @@ SortedDataInterfaceUnique::SortedDataInterfaceUnique(const Ordering& ordering, S
     _KSForIdentEnd = createRadixKeyWithoutLocFromObj(BSONObj(), _identEnd, _ordering);
 }
 
-Status SortedDataInterfaceUnique::insert(OperationContext* opCtx,
-                                         const KeyString::Value& keyString,
-                                         bool dupsAllowed) {
+StatusWith<bool> SortedDataInterfaceUnique::insert(OperationContext* opCtx,
+                                                   const KeyString::Value& keyString,
+                                                   bool dupsAllowed) {
     StringStore* workingCopy(RecoveryUnit::get(opCtx)->getHead());
     RecordId loc = decodeRecordId(keyString, rsKeyFormat());
 
@@ -1378,7 +1378,7 @@ Status SortedDataInterfaceUnique::insert(OperationContext* opCtx,
         auto added = data.add(loc, keyString.getTypeBits());
         if (!added) {
             // Already indexed
-            return Status::OK();
+            return false;
         }
 
         workingCopy->update({std::move(key), *added});
@@ -1387,7 +1387,7 @@ Status SortedDataInterfaceUnique::insert(OperationContext* opCtx,
         workingCopy->insert({std::move(key), *data.add(loc, keyString.getTypeBits())});
     }
     RecoveryUnit::get(opCtx)->makeDirty();
-    return Status::OK();
+    return true;
 }
 
 void SortedDataInterfaceUnique::unindex(OperationContext* opCtx,
@@ -1557,9 +1557,9 @@ SortedDataInterfaceStandard::SortedDataInterfaceStandard(const Ordering& orderin
     _KSForIdentEnd = createMinRadixKeyFromObj(BSONObj(), _identEnd, _ordering);
 }
 
-Status SortedDataInterfaceStandard::insert(OperationContext* opCtx,
-                                           const KeyString::Value& keyString,
-                                           bool dupsAllowed) {
+StatusWith<bool> SortedDataInterfaceStandard::insert(OperationContext* opCtx,
+                                                     const KeyString::Value& keyString,
+                                                     bool dupsAllowed) {
     StringStore* workingCopy(RecoveryUnit::get(opCtx)->getHead());
     RecordId loc = decodeRecordId(keyString, rsKeyFormat());
 
@@ -1569,7 +1569,7 @@ Status SortedDataInterfaceStandard::insert(OperationContext* opCtx,
             .second;
     if (inserted)
         RecoveryUnit::get(opCtx)->makeDirty();
-    return Status::OK();
+    return inserted;
 }
 
 void SortedDataInterfaceStandard::unindex(OperationContext* opCtx,
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.h b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.h
index 7a497c9ea19..70ef2d03363 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.h
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_sorted_impl.h
@@ -118,9 +118,9 @@ public:
     SortedDataInterfaceUnique(const Ordering& ordering, StringData ident);
     std::unique_ptr<SortedDataBuilderInterface> makeBulkBuilder(OperationContext* opCtx,
                                                                 bool dupsAllowed) override;
-    Status insert(OperationContext* opCtx,
-                  const KeyString::Value& keyString,
-                  bool dupsAllowed) override;
+    StatusWith<bool> insert(OperationContext* opCtx,
+                            const KeyString::Value& keyString,
+                            bool dupsAllowed) override;
     void unindex(OperationContext* opCtx,
                  const KeyString::Value& keyString,
                  bool dupsAllowed) override;
@@ -147,9 +147,9 @@ public:
     SortedDataInterfaceStandard(const Ordering& ordering, StringData ident);
     std::unique_ptr<SortedDataBuilderInterface> makeBulkBuilder(OperationContext* opCtx,
                                                                 bool dupsAllowed) override;
-    Status insert(OperationContext* opCtx,
-                  const KeyString::Value& keyString,
-                  bool dupsAllowed) override;
+    StatusWith<bool> insert(OperationContext* opCtx,
+                            const KeyString::Value& keyString,
+                            bool dupsAllowed) override;
     void unindex(OperationContext* opCtx,
                  const KeyString::Value& keyString,
                  bool dupsAllowed) override;
diff --git a/src/mongo/db/storage/sorted_data_interface.h b/src/mongo/db/storage/sorted_data_interface.h
index efa93c6a9e8..13a44b36c79 100644
--- a/src/mongo/db/storage/sorted_data_interface.h
+++ b/src/mongo/db/storage/sorted_data_interface.h
@@ -95,14 +95,17 @@ public:
      * @param dupsAllowed true if duplicate keys are allowed, and false
      *        otherwise
      *
-     * @return Status::OK() if the insert succeeded,
+     * @return true if the key was inserted
      *
-     *         ErrorCodes::DuplicateKey if 'keyString' already exists in 'this' index
-     *         at a RecordId other than 'loc' and duplicates were not allowed
+     *         false if 'dupsAllowed' is true the exact key (including RecordId) already existed in
+     *         the index
+     *
+     *         ErrorCodes::DuplicateKey if 'dupsAllowed' is false and the prefix key (ignoring
+     *         RecordId) already existed in the index
      */
-    virtual Status insert(OperationContext* opCtx,
-                          const KeyString::Value& keyString,
-                          bool dupsAllowed) = 0;
+    virtual StatusWith<bool> insert(OperationContext* opCtx,
+                                    const KeyString::Value& keyString,
+                                    bool dupsAllowed) = 0;
 
     /**
      * Remove the entry from the index with the specified KeyString, which must have a RecordId
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
index 592e2a10237..a715823679c 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
@@ -273,9 +273,9 @@ void dassertRecordIdAtEnd(const KeyString::Value& keyString, KeyFormat keyFormat
 }
 }  // namespace
 
-Status WiredTigerIndex::insert(OperationContext* opCtx,
-                               const KeyString::Value& keyString,
-                               bool dupsAllowed) {
+StatusWith<bool> WiredTigerIndex::insert(OperationContext* opCtx,
+                                         const KeyString::Value& keyString,
+                                         bool dupsAllowed) {
     dassert(opCtx->lockState()->isWriteLocked());
     dassertRecordIdAtEnd(keyString, _rsKeyFormat);
 
@@ -1487,10 +1487,10 @@ std::unique_ptr<SortedDataInterface::Cursor> WiredTigerIdIndex::newCursor(Operat
     return std::make_unique<WiredTigerIdIndexCursor>(*this, opCtx, forward);
 }
 
-Status WiredTigerIdIndex::_insert(OperationContext* opCtx,
-                                  WT_CURSOR* c,
-                                  const KeyString::Value& keyString,
-                                  bool dupsAllowed) {
+StatusWith<bool> WiredTigerIdIndex::_insert(OperationContext* opCtx,
+                                            WT_CURSOR* c,
+                                            const KeyString::Value& keyString,
+                                            bool dupsAllowed) {
     invariant(!dupsAllowed);
     const RecordId id =
         KeyString::decodeRecordIdLongAtEnd(keyString.getBuffer(), keyString.getSize());
@@ -1513,7 +1513,9 @@ Status WiredTigerIdIndex::_insert(OperationContext* opCtx,
     auto& metricsCollector = ResourceConsumption::MetricsCollector::get(opCtx);
     metricsCollector.incrementOneIdxEntryWritten(keyItem.size);
 
-    if (ret != WT_DUPLICATE_KEY) {
+    if (!ret) {
+        return true;
+    } else if (ret != WT_DUPLICATE_KEY) {
         return wtRCToStatus(ret);
     }
 
@@ -1522,10 +1524,10 @@ Status WiredTigerIdIndex::_insert(OperationContext* opCtx,
         key, _desc->getEntry()->getNSSFromCatalog(opCtx), _indexName, _keyPattern, _collation);
 }
 
-Status WiredTigerIndexUnique::_insert(OperationContext* opCtx,
-                                      WT_CURSOR* c,
-                                      const KeyString::Value& keyString,
-                                      bool dupsAllowed) {
+StatusWith<bool> WiredTigerIndexUnique::_insert(OperationContext* opCtx,
+                                                WT_CURSOR* c,
+                                                const KeyString::Value& keyString,
+                                                bool dupsAllowed) {
     LOGV2_TRACE_INDEX(
         20097, "Timestamp safe unique idx KeyString: {keyString}", "keyString"_attr = keyString);
 
@@ -1595,10 +1597,13 @@ Status WiredTigerIndexUnique::_insert(OperationContext* opCtx,
     metricsCollector.incrementOneIdxEntryWritten(keyItem.size);
 
     // It is possible that this key is already present during a concurrent background index build.
-    if (ret != WT_DUPLICATE_KEY)
-        invariantWTOK(ret);
+    if (ret == WT_DUPLICATE_KEY) {
+        return false;
+    }
 
-    return Status::OK();
+    invariantWTOK(ret);
+
+    return true;
 }
 
 void WiredTigerIdIndex::_unindex(OperationContext* opCtx,
@@ -1732,10 +1737,10 @@ std::unique_ptr<SortedDataBuilderInterface> WiredTigerIndexStandard::makeBulkBui
     return std::make_unique<StandardBulkBuilder>(this, opCtx);
 }
 
-Status WiredTigerIndexStandard::_insert(OperationContext* opCtx,
-                                        WT_CURSOR* c,
-                                        const KeyString::Value& keyString,
-                                        bool dupsAllowed) {
+StatusWith<bool> WiredTigerIndexStandard::_insert(OperationContext* opCtx,
+                                                  WT_CURSOR* c,
+                                                  const KeyString::Value& keyString,
+                                                  bool dupsAllowed) {
     invariant(dupsAllowed);
 
     WiredTigerItem keyItem(keyString.getBuffer(), keyString.getSize());
@@ -1752,13 +1757,16 @@ Status WiredTigerIndexStandard::_insert(OperationContext* opCtx,
     auto& metricsCollector = ResourceConsumption::MetricsCollector::get(opCtx);
     metricsCollector.incrementOneIdxEntryWritten(keyItem.size);
 
-    // If the record was already in the index, we just return OK.
+    // If the record was already in the index, return false.
     // This can happen, for example, when building a background index while documents are being
     // written and reindexed.
-    if (ret != 0 && ret != WT_DUPLICATE_KEY)
+    if (ret == WT_DUPLICATE_KEY) {
+        return false;
+    } else if (ret) {
         return wtRCToStatus(ret);
+    }
 
-    return Status::OK();
+    return true;
 }
 
 void WiredTigerIndexStandard::_unindex(OperationContext* opCtx,
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index.h b/src/mongo/db/storage/wiredtiger/wiredtiger_index.h
index f37605e784f..8c6275593df 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_index.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index.h
@@ -99,9 +99,9 @@ public:
                     const IndexDescriptor* desc,
                     bool readOnly);
 
-    virtual Status insert(OperationContext* opCtx,
-                          const KeyString::Value& keyString,
-                          bool dupsAllowed);
+    virtual StatusWith<bool> insert(OperationContext* opCtx,
+                                    const KeyString::Value& keyString,
+                                    bool dupsAllowed);
 
     virtual void unindex(OperationContext* opCtx,
                          const KeyString::Value& keyString,
@@ -156,10 +156,10 @@ public:
     virtual bool isTimestampSafeUniqueIdx() const = 0;
 
 protected:
-    virtual Status _insert(OperationContext* opCtx,
-                           WT_CURSOR* c,
-                           const KeyString::Value& keyString,
-                           bool dupsAllowed) = 0;
+    virtual StatusWith<bool> _insert(OperationContext* opCtx,
+                                     WT_CURSOR* c,
+                                     const KeyString::Value& keyString,
+                                     bool dupsAllowed) = 0;
 
     virtual void _unindex(OperationContext* opCtx,
                           WT_CURSOR* c,
@@ -219,10 +219,10 @@ public:
     bool isDup(OperationContext* opCtx, WT_CURSOR* c, const KeyString::Value& keyString) override;
 
 protected:
-    Status _insert(OperationContext* opCtx,
-                   WT_CURSOR* c,
-                   const KeyString::Value& keyString,
-                   bool dupsAllowed) override;
+    StatusWith<bool> _insert(OperationContext* opCtx,
+                             WT_CURSOR* c,
+                             const KeyString::Value& keyString,
+                             bool dupsAllowed) override;
 
     void _unindex(OperationContext* opCtx,
                   WT_CURSOR* c,
@@ -270,10 +270,10 @@ public:
     }
 
 protected:
-    Status _insert(OperationContext* opCtx,
-                   WT_CURSOR* c,
-                   const KeyString::Value& keyString,
-                   bool dupsAllowed) override;
+    StatusWith<bool> _insert(OperationContext* opCtx,
+                             WT_CURSOR* c,
+                             const KeyString::Value& keyString,
+                             bool dupsAllowed) override;
 
     void _unindex(OperationContext* opCtx,
                   WT_CURSOR* c,
@@ -310,10 +310,10 @@ public:
     }
 
 protected:
-    Status _insert(OperationContext* opCtx,
-                   WT_CURSOR* c,
-                   const KeyString::Value& keyString,
-                   bool dupsAllowed) override;
+    StatusWith<bool> _insert(OperationContext* opCtx,
+                             WT_CURSOR* c,
+                             const KeyString::Value& keyString,
+                             bool dupsAllowed) override;
 
     void _unindex(OperationContext* opCtx,
                   WT_CURSOR* c,