Skip to content

fix(replication): converge delete-marker purge retries and MRF healing - #184

Closed
julienlau wants to merge 2 commits into
pgsty:mainfrom
julienlau:fix/delete-marker-purge-convergence
Closed

julienlau wants to merge 2 commits into
pgsty:mainfrom
julienlau:fix/delete-marker-purge-convergence

Conversation

@julienlau

@julienlau julienlau commented Sep 14, 2026 •

Copy link
Copy Markdown

Follow-up to #162; completes the delete-marker convergence work (context: #152, #153, and docs/site-replication/delete-marker-convergence.md in this PR).

Problem

#162 fixed the first-attempt classification of delete-marker purges (DeleteObjectHandler, heal and resync paths now send the purge as a version purge) and made MRF drops observable. Three defects remained on the delete-marker path, and they re-create the non-convergence loop described in #153 whenever a delivery fails once (the norm with N>2 sites):

  1. Purge retries short-circuited by creation status. In replicateDeleteToTarget, the "already replicated" early-out tested PrevReplicationStatus (the marker's creation status) even for a purge, so a purge whose first delivery to a site failed was never retried — that site kept the marker forever. Only a manual mc admin replicate resync bypassed it.
  2. Purge outcome recorded in the wrong field. On RemoveObject success for a marker-path purge (VersionID == ""), the code set ReplicationStatus = Completed but never VersionPurgeStatus = VersionPurgeComplete: the source's PurgeTargets stayed PENDING forever, the scanner re-queued the purge every cycle, and the marker version never left xl.meta.
  3. MRF dropped every delete-marker entry. queueMRFHeal calls GetObjectInfo(VersionID=<marker version>), which deterministically answers 405 MethodNotAllowed; the entry was silently continued, so the fastest retry path never ran for deletes.

Residual effect: stuck PENDING states drove perpetual heal/resync traffic whose per-marker probes are answered 405 — the storm from #153 (measured there at ~4,100 req/s, 96.8% answered 405 in an 84M-version mesh).

Fix

In replicateDeleteToTarget, a marker-path purge is identified precisely:

isDMPurge := dobj.VersionID == "" && dobj.DeleteMarkerVersionID != "" && !rinfo.VersionPurgeStatus.Empty()
  • the early-out is guarded with !isDMPurge and the marker's creation status is pinned (rinfo.ReplicationStatus = rinfo.PrevReplicationStatus) so purges neither get suppressed by nor overwrite it;
  • the offline / error / success branches record purge outcomes in VersionPurgeStatus whenever dobj.VersionID != "" || isDMPurge — the composite reaches VersionPurgeComplete, the source rewrite takes the xlMetaV2.DeleteVersion removal path and the marker version is dropped;
  • queueMRFHeal accepts the 405 (it carries a fully populated ObjectInfo) and routes the entry through QueueReplicationHeal → the (now fixed) delete path.

405 remains "marker already replicated" for creations only — same semantics #162 preserved; what disappears is the unbounded repetition driven by states that could never advance.

Tests

  • TestReplicateDeleteMarkerTargetSemantics/marker-path_purge_* (new): purge in the legacy shape with PrevReplicationStatus=COMPLETED, VersionPurgeStatus=PENDING must not short-circuit; success yields VersionPurgeComplete with the creation status preserved; rejection yields VersionPurgeFailed.
  • TestReplicateDeleteMarkerPurge/recover_legacy_true (reworked): a legacy-shaped purge now converges on the first proper delivery — marker version dropped on source and target, nothing further scheduled — instead of documenting the pre-fix PENDING state and recovering via a heal re-queue.
  • Full TestReplication|TestReplicate|TestMRF|TestResync|TestSiteResync surface (23 tests) plus delete-handler tests pass; go build ./..., go vet, gofmt clean.

Docs

docs/site-replication/delete-marker-convergence.md: port of the convergence write-up with silo line references, a status note on what #162 covered, the applied fix, reproduction (3 sites) and known remaining limitations (wire tombstones, non-attempted targets, replica markers, per-code 405 metrics, MRF backoff re-enqueue — follow-ups from #152/#153).

Fixes the convergence loop reported in #153 (retry/purge-status side; the observability suggestions from #152/#153 are listed as follow-ups in the doc).

Companion write-up

Blog post covering the investigation and both fixes (#162, #184), to be published at: https://pepitedata.com/minio-silo-delete-marker-convergence-fix/

replicateDeleteToTarget short-circuited a marker purge riding the
delete-marker path (VersionID empty) with the marker's creation status,
and recorded its outcome in ReplicationStatus instead of
VersionPurgeStatus, so a purge that failed its first delivery was never
retried and never finalized: the source stayed PENDING forever, the
scanner re-queued it on every cycle, and the purged marker version
lingered in xl.meta.

Classify the purge from its tracked state (isDMPurge): never short-
circuit it with the creation status, never overwrite that status, and
record purge outcomes in VersionPurgeStatus so the composite reaches
COMPLETE and the source rewrite takes the version-removal path.

queueMRFHeal dropped every delete-marker entry because GetObjectInfo on
a marker version answers MethodNotAllowed; accept that error and route
the returned ObjectInfo back through the delete replication path, so
failed delete replications are healed by the MRF instead of waiting for
the scanner.

Completes the delete-marker convergence work started in pgsty#162 (which
fixed the first-attempt classification in DeleteObjectHandler and MRF
observability); see pgsty#152, pgsty#153.
Port the delete-marker convergence write-up with silo line references
and a status note on what pgsty#162 already covered and what remains.
@julienlau
julienlau marked this pull request as ready for review September 14, 2026 17:49
@Vonng

Vonng commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Hi @julienlau, thank you for the careful investigation and for putting together this PR, including the proposed fix, tests, and documentation. I'm sorry my earlier response was too abrupt and did not properly acknowledge the work you put into it.

Although we did not merge this PR directly, your contribution helped shape the fix that landed in #196 (40220bd836cb). Your analysis of purge-state handling and delete-marker recovery helped us identify and address gaps left after #162.

Recognizing your contribution

We gratefully acknowledge Julien Laurenceau (@julienlau) for reporting these issues and contributing the proposed solution, regression tests, and documentation in #184. We recognize this work as a valuable contribution to SILO and to the resulting repair, even though the final implementation was merged through a separate PR. Thank you for sharing your findings and helping us improve replication reliability.

We have also documented the related investigation and follow-up on the SILO website:

The fix is on main; final release acceptance is tracked in #203.

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.

3 participants