SERVER-130130: fix(sharding): model delete for deferred xferMods update that removes an in-range doc

src/mongo/db/s/migration_chunk_cloner_source.cpp

@@ -909,9 +909,29 @@ void MigrationChunkClonerSource::_processDeferredXferMods(OperationContext* opCt
         auto idElement = preImageDocKey["_id"];
         BSONObj newerVersionDoc;
         if (!Helpers::findById(opCtx, this->nss(), BSON("_id" << idElement), newerVersionDoc)) {
-            // If the document can no longer be found, this means that another later op must have
-            // deleted it. That delete would have been captured by the xferMods so nothing else to
-            // do here.
+            // The document can no longer be found, which means that a later operation deleted it.
+            //
+            // If the document's shard key was still inside the chunk range when it was deleted,
+            // onDeleteOp() captured that delete in the xferMods delete buffer and there is nothing
+            // else to do here. However, the deferred update we are processing here may itself have
+            // moved the document's shard key OUT of the chunk range before the delete happened. In
+            // that case onDeleteOp() would have observed an out-of-range shard key and skipped the
+            // delete, so it was never added to the xferMods buffer. Because the document was
+            // transferred to the recipient while its pre-image was still in range, simply skipping
+            // it here would leave the recipient with an orphaned copy that no longer exists on the
+            // donor.
+            //
+            // To guarantee the recipient does not retain such an orphan, model the deletion
+            // ourselves whenever the pre-image fell inside the chunk range. Emitting a delete here
+            // is safe even if onDeleteOp() did capture the delete: the recipient applies deletes by
+            // _id and the orphan counter is only adjusted when a document is actually removed, so a
+            // duplicate delete is an idempotent no-op.
+            auto preImageShardKeyValues =
+                _shardKeyPattern.extractShardKeyFromDocumentKey(preImageDocKey);
+            if (!preImageShardKeyValues.isEmpty() &&
+                isKeyInRange(preImageShardKeyValues, getMin(), getMax())) {
+                _addToTransferModsQueue(idElement.wrap(), 'd', {});
+            }
             continue;
         }
 

src/mongo/db/s/migration_chunk_cloner_source.h

@@ -420,6 +420,10 @@ class MigrationChunkClonerSource {
     friend class LogTransactionOperationsForShardingHandler;
     friend class LogBatchedWriteForSessionMigrationHandler;
 
+    // Allows the unit test to exercise the deferred-xferMods reconciliation path, which is
+    // otherwise only reachable through the transaction op-observer handler.
+    friend class MigrationChunkClonerSourceTest;
+
     using RecordIdSet = std::set<RecordId>;
 
     /**

src/mongo/db/s/migration_chunk_cloner_source_test.cpp

@@ -1600,6 +1600,65 @@ TEST_F(MigrationChunkClonerSourceTest, UpdatedDocumentsFetched) {
     cloner.cancelClone(operationContext());
 }
 
+// Regression test for the deferred-xferMods reconciliation path. When a transaction's update is
+// processed without a post-image document key (e.g. a transaction prepared in a previous term), the
+// cloner defers the work and only records the pre-image document key. Later, when nextModsBatch()
+// reconciles the deferred entry, the document may already be gone because a subsequent update moved
+// its shard key out of the chunk range and it was then deleted. That out-of-range delete is skipped
+// by onDeleteOp(), so unless the deferred reconciliation models the delete itself, the recipient is
+// left with an orphaned copy of the document that was transferred while its pre-image was in range.
+TEST_F(MigrationChunkClonerSourceTest, DeferredUpdateForRemovedInRangeDocModelsDelete) {
+    const ShardKeyPattern shardKeyPattern(kShardKeyPattern);
+
+    const ShardsvrMoveRange req =
+        createMoveRangeRequest(ChunkRange(BSON("X" << 100), BSON("X" << 200)));
+    MigrationChunkClonerSource cloner(operationContext(),
+                                      req,
+                                      WriteConcernOptions(),
+                                      kShardKeyPattern,
+                                      kDonorConnStr,
+                                      kRecipientConnStr.getServers()[0]);
+
+    // Materialize the collection with an unrelated in-range document so that findById() lookups
+    // resolve against an existing collection. This document is never queued for cloning and is not
+    // part of any deferred entry, so it must not appear in the transferred mods.
+    insertDocsInShardedCollection({createCollectionDocument(175)});
+
+    // Defer reconciliation for a document whose pre-image is inside the chunk range, but which no
+    // longer exists in the collection (it was moved out of range and subsequently deleted). The
+    // document key carries both the shard key and the _id, matching what the op-observer records.
+    cloner._deferProcessingForXferMod(createCollectionDocument(150));
+
+    // Also defer a document whose pre-image is *outside* the chunk range. The recipient never
+    // received this document, so no delete should be modeled for it.
+    cloner._deferProcessingForXferMod(createCollectionDocument(90));
+
+    {
+        const auto collection = acquireCollection(operationContext(), kNss, MODE_IS);
+
+        {
+            BSONArrayBuilder arrBuilder;
+            ASSERT_OK(cloner.nextCloneBatch(operationContext(), collection, &arrBuilder));
+            ASSERT_EQ(0, arrBuilder.arrSize());
+        }
+
+        {
+            BSONObjBuilder modsBuilder;
+            ASSERT_OK(cloner.nextModsBatch(operationContext(), &modsBuilder));
+
+            const auto modsObj = modsBuilder.obj();
+            ASSERT_EQ(0U, modsObj["reload"].Array().size());
+
+            // Only the in-range pre-image is reconciled into a delete; the out-of-range pre-image is
+            // ignored because the recipient never held that document.
+            ASSERT_EQ(1U, modsObj["deleted"].Array().size());
+            ASSERT_BSONOBJ_EQ(BSON("_id" << 150), modsObj["deleted"].Array()[0].Obj());
+        }
+    }
+
+    cloner.cancelClone(operationContext());
+}
+
 TEST_F(MigrationChunkClonerSourceTest, UpdatedDocumentsFetchedWithHashedShardKey) {
     const ShardKeyPattern shardKeyPattern(BSON("X" << "hashed"));
 

src/mongo/db/s/migration_destination_manager.cpp

@@ -2089,16 +2089,22 @@ bool MigrationDestinationManager::_applyMigrateOp(OperationContext* opCtx, const
                 }
             }
 
-            writeConflictRetry(opCtx, "transferModsDeletes", _nss, [&] {
-                deleteObjects(opCtx,
-                              collection,
-                              id,
-                              true /* justOne */,
-                              false /* god */,
-                              true /* fromMigrate */);
+            const auto numDeleted = writeConflictRetry(opCtx, "transferModsDeletes", _nss, [&] {
+                return deleteObjects(opCtx,
+                                     collection,
+                                     id,
+                                     true /* justOne */,
+                                     false /* god */,
+                                     true /* fromMigrate */);
             });
 
-            changeInOrphans--;
+            // Only adjust the orphan counter when a document was actually removed. The donor may
+            // legitimately send a delete for an _id that is no longer present on the recipient (for
+            // example a delete that is redundant with a deferred-update reconciliation), and
+            // decrementing for a no-op delete would corrupt the persisted orphan count.
+            if (numDeleted > 0) {
+                changeInOrphans--;
+            }
             didAnything = true;
         }
     }