Skip to content

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

Open
Abuhaithem wants to merge 1 commit into
mongodb:masterfrom
Abuhaithem:fix/sharding-deferred-xfermods-missed-delete-orphan
Open

SERVER-130130: fix(sharding): model delete for deferred xferMods update that removes an in-range doc#1820
Abuhaithem wants to merge 1 commit into
mongodb:masterfrom
Abuhaithem:fix/sharding-deferred-xfermods-missed-delete-orphan

Conversation

@Abuhaithem

@Abuhaithem Abuhaithem commented Jun 29, 2026

Copy link
Copy Markdown

Fixes: SERVER-130130

Fixes a missed-delete in chunk migration that can leave the recipient shard with an orphaned
document
that no longer exists on the donor.

When a transaction's update is deferred by the donor cloner (because the post-image document key
is absent from the oplog entry — e.g. a transaction prepared in a previous term that commits during a
migration), MigrationChunkClonerSource::_processDeferredXferMods() reconciles it later by reading
the current document. If the document is no longer present, the existing code assumes a later delete
was already captured and skips the entry:

// 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.
continue;

That assumption is wrong when the deferred update moved the document's shard key out of the chunk
range
before it was deleted: onDeleteOp() drops out-of-range deletes, so the delete is never
transferred — yet the recipient already received the document while its pre-image was in range. The
recipient keeps the document as an orphan, silently resurrecting it once the migration commits.

Changes

  • Donor (migration_chunk_cloner_source.cpp) — in the deferred-reconciliation "not found"
    branch, model a delete for the _id whenever the pre-image shard key was inside the chunk
    range. This is precise (no delete is sent for documents the recipient never held) and idempotent
    (a redundant delete-by-_id removes nothing).
  • Recipient (migration_destination_manager.cpp) — decrement the persisted orphan counter only
    when deleteObjects() actually removed a document. This keeps orphan accounting correct in the
    presence of (now possible) redundant deletes and also fixes a pre-existing latent over-count.
  • Test (migration_chunk_cloner_source_test.cpp) — new regression test
    DeferredUpdateForRemovedInRangeDocModelsDelete.
  • Header (migration_chunk_cloner_source.h) — befriend the test fixture so the deferred path,
    otherwise only reachable through the transaction op-observer handler, can be exercised
    deterministically in a unit test.

Why it's correct

  • findById returning not found means the _id is absent on the donor, so the recipient must not
    retain it.
  • Gating on the pre-image range means a delete is emitted only for documents the recipient could
    have received.
  • A duplicate delete (if onDeleteOp did capture an in-range delete) is a no-op on the recipient and
    no longer mis-counts orphans thanks to the recipient-side change.

Testing

  • bazel test //src/mongo/db/s:migration_chunk_cloner_source_test
    • New: DeferredUpdateForRemovedInRangeDocModelsDelete
    • Existing UpdatedDocumentsFetched* and RemoveDuplicateDocuments continue to pass (non-deferred
      behavior unchanged).

Risk

Low. The donor change is confined to the deferred-update branch (prior-term prepared transactions
during migration); the recipient change is a one-line guard around an existing decrement using a
value deleteObjects() already returns. No wire-protocol or on-disk format change.

…te that removes an in-range doc

When a chunk migration defers a transaction update because the post-image
document key is missing from the oplog (e.g. a transaction prepared in a
previous term), the donor reconciles it later in _processDeferredXferMods().
If the document is no longer present, the code assumed a later delete had
already been captured by xferMods and skipped the entry. That assumption fails
when the deferred update moved the document's shard key out of the chunk range
before it was deleted: onDeleteOp() drops the out-of-range delete, so it is
never transferred and the recipient is left with an orphaned copy of a document
that no longer exists on the donor.

Model a delete for the document's _id whenever its pre-image was in range, and
on the recipient only decrement the persisted orphan counter when a document is
actually removed, so the now-possible redundant delete stays idempotent.
@kelly-cs

kelly-cs commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Hi @Abuhaithem - thanks for the SERVER ticket and pull request.

As part of considering the PR, I noticed you signed our contributor's agreement. However, I think your github username is missing from it. Can you please try to re-sign the agreement with your Abuhaithem username? It looks like that field got populated with a date or something instead.

Separately I am going to get this in front of the team the works on this component for consideration. For us to actively consider your provided code we'll need an updated contributor's agreement signed, but we'll continue to look at the problem wholistically on our own.

@Abuhaithem

Copy link
Copy Markdown
Author

Hi @Abuhaithem - thanks for the SERVER ticket and pull request.

As part of considering the PR, I noticed you signed our contributor's agreement. However, I think your github username is missing from it. Can you please try to re-sign the agreement with your Abuhaithem username? It looks like that field got populated with a date or something instead.

Separately I am going to get this in front of the team the works on this component for consideration. For us to actively consider your provided code we'll need an updated contributor's agreement signed, but we'll continue to look at the problem wholistically on our own.

Hi @kelly-cs !

Thanks for catching that error. I have just re-signed the contributor's agreement and made sure my GitHub username (@Abuhaithem) is in the correct field this time. Let me know if everything looks good on your end now!

@kelly-cs

kelly-cs commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thank you! That should cover your other PR #1819 as well. We'll take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants