Conversation
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
marked this pull request as ready for review
September 14, 2026 17:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #162; completes the delete-marker convergence work (context: #152, #153, and
docs/site-replication/delete-marker-convergence.mdin 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):replicateDeleteToTarget, the "already replicated" early-out testedPrevReplicationStatus(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 manualmc admin replicate resyncbypassed it.RemoveObjectsuccess for a marker-path purge (VersionID == ""), the code setReplicationStatus = Completedbut neverVersionPurgeStatus = VersionPurgeComplete: the source'sPurgeTargetsstayedPENDINGforever, the scanner re-queued the purge every cycle, and the marker version never leftxl.meta.queueMRFHealcallsGetObjectInfo(VersionID=<marker version>), which deterministically answers 405 MethodNotAllowed; the entry was silentlycontinued, so the fastest retry path never ran for deletes.Residual effect: stuck
PENDINGstates 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:!isDMPurgeand the marker's creation status is pinned (rinfo.ReplicationStatus = rinfo.PrevReplicationStatus) so purges neither get suppressed by nor overwrite it;VersionPurgeStatuswheneverdobj.VersionID != "" || isDMPurge— the composite reachesVersionPurgeComplete, the source rewrite takes thexlMetaV2.DeleteVersionremoval path and the marker version is dropped;queueMRFHealaccepts the 405 (it carries a fully populatedObjectInfo) and routes the entry throughQueueReplicationHeal→ 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 withPrevReplicationStatus=COMPLETED, VersionPurgeStatus=PENDINGmust not short-circuit; success yieldsVersionPurgeCompletewith the creation status preserved; rejection yieldsVersionPurgeFailed.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-fixPENDINGstate and recovering via a heal re-queue.TestReplication|TestReplicate|TestMRF|TestResync|TestSiteResyncsurface (23 tests) plus delete-handler tests pass;go build ./...,go vet,gofmtclean.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/