fix(replication): drain priority sync queue and recover from routing-event lag#165
Open
mickvandijke wants to merge 9 commits into
Open
fix(replication): drain priority sync queue and recover from routing-event lag#165mickvandijke wants to merge 9 commits into
mickvandijke wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves replication convergence under churn by (a) ensuring neighbor sync drains queued priority peers without waiting for periodic ticks and (b) recovering from lagged DHT routing-table broadcast events. It also expands verification/fetch pipeline behavior (batching caps, retry/defer semantics, paid-list edge-voter quorum handling) and adds an e2e scenario covering paid-list-authorized repair below storage quorum.
Changes:
- Neighbor-sync loop now drains the priority queue back-to-back and resyncs close peers on
RecvError::Lagged. - Verification/fetch pipeline enhancements: bounded verification batches, fetch→verification retry metadata, and deferred re-verification scheduling.
- Paid-list quorum evaluation updated with “edge voter” handling; adds a deterministic paid-list repair e2e scenario and bumps crate version.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/poc_d1_bounded_queues.rs | Updates test VerificationEntry construction for new timing fields. |
| tests/poc_bootstrap_stall.rs | Updates test VerificationEntry construction for new timing fields. |
| tests/e2e/replication.rs | Adds deterministic e2e scenario for paid-list-majority repair under low storage quorum. |
| src/replication/types.rs | Adds next_verify_at, fetch retry metadata, and NeighborSyncState::has_priority_peers + test. |
| src/replication/scheduling.rs | Adds deferred pending scheduling, fetch retry→verification requeue, and returns evicted keys. |
| src/replication/quorum.rs | Adds edge-aware paid-list vote summary and splits verification requests into capped batches. |
| src/replication/mod.rs | Implements lag recovery, neighbor-sync drain-before-park, verification request caps, and retry/defer integration. |
| src/replication/config.rs | Introduces PAID_LIST_FLEX_EDGE_COUNT and MAX_VERIFICATION_KEYS_PER_REQUEST. |
| src/replication/bootstrap.rs | Updates test VerificationEntry construction for new timing fields. |
| src/replication/admission.rs | Adjusts cross-set dedup/admission so paid hints can survive replica rejection under churn. |
| Cargo.toml | Bumps version to 0.14.3. |
| Cargo.lock | Updates locked crate version to 0.14.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
2543
to
2547
| results.push(protocol::KeyVerificationResult { | ||
| key: *key, | ||
| present, | ||
| present: cached.present.unwrap_or(false), | ||
| paid, | ||
| }); |
| } | ||
| continue; | ||
| } | ||
| Err(RecvError::Closed) => continue, |
Comment on lines
+495
to
+497
| let handles = | ||
| spawn_verification_batch_tasks(targets, p2p_node, config.verification_request_timeout); | ||
| collect_verification_batch_results(handles, targets, &mut evidence).await; |
Comment on lines
+43
to
+47
| /// Number of furthest paid-list close-group peers treated as churny edge | ||
| /// voters. | ||
| /// | ||
| /// Once the paid-list close group reaches [`PAID_LIST_CLOSE_GROUP_SIZE`], edge | ||
| /// peers are queried, but a negative edge paid-list response does not count |
SemVer: patch
…event lag Neighbor-sync paid-list hint propagation could stall for tens of minutes on the furthest close-group members during correlated topology change (partition heal, mass join). Two root causes: - The neighbor-sync loop parked on the periodic 10-20 min tick after every single round, and `sync_trigger` is a coalescing `Notify` that collapses a burst of entrant wakeups into one. A churn burst that queued many priority peers therefore drained only one batch (<=4) promptly; the rest waited for subsequent ticks. Park only when `priority_order` is empty and otherwise run rounds back-to-back, draining the durable queue at round-trip speed. The drain terminates because `select_next_sync_peer` pops each priority peer unconditionally and `new_cycle` only refills under `is_cycle_complete()`. - The DHT event handler discarded broadcast `RecvError::Lagged`, silently dropping `KClosestPeersChanged` events under load -- exactly when churn is heaviest -- so missed entrants were never queued. On lag, resynchronize from ground truth: snapshot the current close-peer set, queue every member for priority sync, and fire the trigger. Add `NeighborSyncState::has_priority_peers` and a regression test covering the loop's drain/termination contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SemVer: patch Consume dequeued fetch candidates through reservation-aware queue APIs and requeue no-source retry candidates for verification. Centralize pending_verify insertion bookkeeping so per-sender counters stay in lockstep.
mickvandijke
force-pushed
the
fix/neighbor-sync-drain-window
branch
from
1a6fe63 to
fc8d724
Compare
Comment on lines
+1284
to
+1288
| warn!( | ||
| "Prune audit challenge for {} against {peer} could not acquire coordinator slot", | ||
| hex::encode(key) | ||
| ); | ||
| return PruneAuditChallengeResult::MalformedResponse; |
Comment on lines
+2837
to
2841
| if cached.present.is_none() && paid.is_none() { | ||
| continue; | ||
| } | ||
|
|
||
| results.push(protocol::KeyVerificationResult { |
Comment on lines
18
to
21
| pub mod audit; | ||
| pub mod audit_coordinator; | ||
| pub(crate) mod audit_metrics; | ||
| pub mod bootstrap; |
Comment on lines
+132
to
+134
| fn fresh_offer_key_lock_index(key: &XorName) -> usize { | ||
| usize::from(key[0]) % FRESH_OFFER_KEY_LOCK_SHARDS | ||
| } |
Comment on lines
+1283
to
+1289
| let Some(_slot) = audit_challenge_coordinator.acquire(*peer).await else { | ||
| warn!( | ||
| "Prune audit challenge for {} against {peer} could not acquire coordinator slot", | ||
| hex::encode(key) | ||
| ); | ||
| return PruneAuditChallengeResult::MalformedResponse; | ||
| }; |
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.
Summary
This PR hardens the replication repair path under churn and load. It expands the original neighbor-sync fix into a full set of paid-list repair, verification retry, queue-capacity, and audit-challenge fixes so legitimate repair work is not silently dropped or converted into false audit failures.
The branch contains six commits:
12e8c42-fix(replication): harden paid-list verification repair42d3242-fix(replication): tolerate paid-list edge churne2967a6-fix(replication): drain priority sync queue and recover from routing-event lagad5f9ce-fix(replication): preserve verification retry capacity47f67ae-fix(replication): eliminate false audit challenge timeoutsfc8d724-fix(replication): preserve dequeued retry reservationsProblem
Several replication paths could lose valid work during the exact conditions where repair pressure is highest:
sync_triggeris a coalescingNotify, so many topology-change wakeups can collapse into one. The loop previously ran one round and then parked on the 10-20 minute periodic tick, leaving queued priority peers waiting for later ticks.KClosestPeersChangedbroadcast events meant close-group entrants were never inserted into the priority neighbor-sync queue.pending_verifyinto fetch no longer counted against pending capacity, so unrelated fresh hints could consume the sender/global slots required to requeue that verified key after fetch-source exhaustion.AuditChallenges to the same target and exceed the responder admission cap. Those local bursts were then observed as remote non-responses.Changes
Paid-list verification and repair
next_verify_atto pending verification entries and usesready_pending_keys/defer_pendingso inconclusive or no-holder states wait for a retry delay instead of spinning or being dropped.MAX_VERIFICATION_KEYS_PER_REQUESTand splits outgoing per-peer requests with paid-list indices rebased per batch.Paid-list edge churn handling
PAID_LIST_FLEX_EDGE_COUNT = 4for the furthest paid-list close-group positions.VerificationTargets.ant-nodefrom0.14.2to0.14.3.Neighbor-sync burst and lag recovery
NeighborSyncStatepriority peers back-to-back and only parks whenpriority_orderis empty.NeighborSyncState::has_priority_peersas the loop's durable pending-work signal.RecvError::Laggedfrom the DHT event broadcast by snapshotting the current close-neighbor set, queueing those peers for priority sync, and triggering the loop.RecvError::Closedbehavior unchanged.Verification retry capacity
VerificationEntryretry metadata frompending_verifyintoFetchCandidateand thenInFlightEntry.fetch_queueorin_flight_fetch.pending_verify.verification_request_timeoutinstead of losing the repair work.Dequeued retry reservations
start_dequeued_fetchto transfer a dequeued retry reservation intoin_flight_fetch.discard_fetch_candidateto release a reservation when a dequeued candidate is intentionally dropped.requeue_candidate_for_verificationfor dequeued candidates that have no usable sources; those are restored to pending verification with a retry delay instead of being dropped.pending_verifyand per-sender counters stay in lockstep.Audit challenge timeout hardening
AuditChallengeCoordinator, a shared per-target outbound limiter for local audit issuers.AuditChallenges per target peer, matching the deployed responder per-source digest admission cap.Commit Details
12e8c42- harden paid-list verification repairThis commit makes the paid-list verification pipeline retry-safe. It fixes cross-set hint admission so a paid hint can still authorize metadata convergence when the replica side was rejected by local routing churn. It adds verification retry timing, request batch limits, response scoping, lookup caching, and non-terminal retry behavior for verified keys that currently have no responding holder. It also carries verification context into fetch so exhausted verified repairs can return to pending verification.
42d3242- tolerate paid-list edge churnThis commit makes paid-list majority evaluation edge-aware. The furthest four paid-list peers in a full 20-peer group are queried, but their negative or missing responses are treated as churny boundary disagreement. Positive edge responses still count. This lets stable core paid-list majorities authorize repair without letting negative boundary votes create false failures.
e2967a6- drain priority sync queue and recover from routing-event lagThis commit fixes the original neighbor-sync stall. Priority peers are now drained at round-trip speed instead of one batch per
Notifywakeup, and dropped DHT broadcast events trigger a close-neighbor resnapshot so missed entrants still get priority sync.ad5f9ce- preserve verification retry capacityThis commit makes retryable verified fetches reserve the pending capacity they need to come back. A sender at capacity can no longer have a promoted verified key blocked from requeueing by newer hints from the same sender. The reservation follows the key through fetch queue and in-flight fetch, then releases on completion or requeue.
47f67ae- eliminate false audit challenge timeoutsThis commit prevents local audit bursts from creating false remote timeout verdicts. It adds the shared audit challenge coordinator, wires it through responsible audits, prune audits, and possession checks, and adds local metrics/classification so timeout-like outcomes can be distinguished from unreachable transport failures in logs and counters.
fc8d724- preserve dequeued retry reservationsThis commit closes the remaining reservation hole after dequeue. Fetch workers now consume candidates by value so retry metadata cannot be orphaned. No-source retry candidates are restored to pending verification with a delay, and pending insertion accounting is centralized to keep per-sender counters correct.
Coverage Added Or Updated
scenario_26_paid_list_majority_repairs_missing_replica_below_storage_quorum.Expected Effect
SemVer
Patch.