Second review round: every iam-item / bucket-meta delivery shares a
constant path, so any later successful single-item delivery (a Bob
update) dequeued the escalated marker recording a possibly-unreplayed
deletion (a failed Alice delete) while the entity still existed
remotely. Ordinary settlement now skips escalated entries; only the
repair path — the operator's explicit accountability transfer — clears
them via dequeue_..._including_escalated. A new hook failure still
overwrites the marker and re-arms the drain. Regression covers
survive-ordinary-dequeue and repair-clears.
Review: the bootstrap-plan snapshot cannot replay deletions — a deleted
IAM entity or absent bucket config produces no task, so clearing the
collapsed iam-item / bucket-meta entry after a successful snapshot
resend silently lost a failed delete and the peer kept stale state
permanently.
The drain now keeps those entries until remote absence is proven:
after a successful snapshot resend the entry is escalated
(failed=true, marker last_error) so it stays operator-visible in
retry_stats, and classification skips marked entries so the
once-per-failure-episode snapshot is not re-sent daily. A newer hook
failure overwrites the marker and re-arms the drain; a later full
delivery or a manual repair settles the entry. Escalation is
conditional on the snapshot timestamp, preserving the earlier
review's in-flight-failure guarantee. Bucket ops and peer edits keep
auto-settle — their replays are faithful.
Adversarial review of the drain surfaced one real race and three cheap
hardenings:
- Conditional settlement for collapsed (constant-path) iam-item /
bucket-meta entries: the snapshot resend proves delivery of the state
as of plan-build time, so a hook failure stamped during the delivery
window (a newer local commit the snapshot did not contain) must
survive the snapshot's success instead of being cleared — previously
the peer would silently diverge until the next same-path failure.
(The operator repair path shares this collapse but keeps its existing
unconditional settle; the drain runs every tick and needed the guard
first.)
- Re-check the pending_* gates on the freshly loaded state: an endpoint
refresh can commit its pending marker mid-tick without the lifecycle
guard.
- Do not upsert retry events for peers that already left the state —
remove_sites pruned their entries and they can never drain again.
- Correct the mutual-exclusion comment: the repair-execution lock does
NOT close the dry-run -> execute window (dry-run takes no lock); that
window fails safe via the preflight token, which hashes the
replayable entries.
New tests pin the (Some settled, None failed) generation-settle
combination the peer-edit drain depends on, and the snapshot-relative
conditional settlement.
The retry queue recorded every failed peer delivery but had no
consumer — missed IAM/bucket metadata updates drifted until an operator
ran a manual repair (backlog#1675 P1-3). The 600s reconcile tick now
drains the queue behind the existing lifecycle guard and pending_*
gates.
Replay discipline (pinned by the red-light tests):
- IAM / bucket-meta entries collapse per (peer, path) and their bodies
are not persisted, so the only faithful replay is the current
bootstrap-plan snapshot (SiteReplicationRepairTask reuse) sent to the
affected peer.
- make-with-versioning / configure-replication ops are re-derived from
the CURRENT plan for their bucket — never the recorded path, whose
query can carry an expired one-shot bootstrap token; an op whose
bucket left the plan is provably stale and is settled.
- Peer edits are re-sent as the current peer records under a freshly
allocated generation inside one state transaction — the recorded
generation is stale by definition and the receiver would fence it.
- Destructive bucket ops (delete-bucket / force-delete-bucket) and
internal: marker records (pending-endpoint-refresh backup store) are
never background-replayed.
- Exponential backoff (600s * 2^(n-1), 24h ceiling) gates each attempt;
an unreachable peer's transport failure re-queues its events so the
backoff still advances.
Concurrency: the drain takes the repair-execution config lock — the
operator repair preflight token hashes the replayable retry events, so
settling them between dry-run and execute would strand the operator on
a stale preflight. Lock order matches repair (lifecycle guard -> repair
execution lock -> state object lock); success/failure settlement reuses
the generation-fenced upsert/settle semantics from #5882/#6097.
Scope note: the drain settles 'delivered once and failed' entries. A
hook that never fired (crash between local commit and send) leaves no
entry; a low-frequency plan-diff catch-all remains follow-up work, and
manual repair stays authoritative.
Red-light evidence for backlog#1675 P1-3: the retry queue has no
background consumer — every failed peer delivery waits for a manual
repair. The new tests specify the drain rules before the drain exists:
- classification: IAM / bucket-meta entries collapse per (peer, path)
with no persisted body, so only a bootstrap-plan snapshot resend is a
faithful replay; make-with-versioning / configure-replication are
re-derivable per bucket; peer edits are re-sent under a fresh
generation; destructive bucket ops and internal: marker records
(pending-endpoint-refresh backup store) are never background-replayed
- exponential backoff (600s * 2^(n-1), 24h ceiling) gates every
attempt, otherwise a dead peer's entries hit the failed threshold
within 30 minutes of reconcile ticks
- the actionable subset respects classification, current peer
membership and backoff
All fail against the placeholder implementations (no consumer).
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Zhengchao An