Repository navigation
Conversation
A fetch responder admits two outstanding requests per requesting peer and twelve overall, and refuses the rest at admission without a reply. The requester then waits its whole 30 s fetch timeout, and saorsa-core charges the responder a connection-timeout trust event it did not earn. As measured for WithAutonomi#252 from the responders' cumulative admission counters, this was 36.6% of inbound fetch requests on 0.21.0 and 44.3% on 0.22.0, and the WARN line counted 3.38M refusals in the 25 hours of the 2026-10-11 join wave. A requester now says it understands a busy reply, and a responder that refuses such a request answers `FetchResponse::Busy` at once, within bounds. Wire: `FetchRequest` keeps its serialized layout. A requester that understands busy replies appends a 5-byte capability trailer ("antf" and a flags byte) after the postcard message. Older nodes decode with `from_bytes`, which ignores it, so they refuse silently as before. A current node reads it with `take_from_bytes`. `FetchResponse::Busy` is appended after the existing variants, whose discriminants are unchanged, and is only sent in answer to a request with the trailer, so an older requester never receives it. Responder: a busy reply needs a free send permit (48), the per-peer allowance (two back to back, then one a second, at most two in flight) and the global one (32 back to back, then 16 a second; every message is signed, so about 5 KiB each). It is sent off the serial message loop, abandoned after 2 s and raced against shutdown. Anything outside those bounds is the old silence. The refusal WARN line keeps its text and counters and gains `capability` and `reply` fields; new cumulative counters report each busy-reply outcome. Requester: the fetch worker's requests ask for busy replies. A busy answer for the requested key carries no trust event. The attempt waits 5-10 s (jittered, never past its own fetch timeout) while holding its fetch slot, then ends as a source failure, so the queue moves on exactly as after a timeout. The busy holder is put last when choosing where to start or retry a key while its cooldown runs: the wait, doubling with each further busy answer up to 60 s. Serving once resets that doubling, and so do 10 minutes without a busy answer. Merge after WithAutonomi#252: its per-source share removes most per-peer over-share, and the refusals that remain are what this answers. A test-only fetch serve gate, the same one WithAutonomi#252 adds, makes the end-to-end tests deterministic. ADR-0025 records the decision. BREAKING CHANGE: `replication::protocol::FetchRequest` gains a public `busy_reply` field (not serialized) and `FetchResponse` gains a `Busy` variant, so code constructing the struct or matching the enum exhaustively must be updated. The wire format stays compatible with older nodes in both directions.
`SourceBusyTracker::record_busy` never shortens a cooldown that is already running, but it returned the newly computed cooldown, so when an earlier, longer one was still running the requester's debug line reported less than the holder was really put last for. It now returns what is left of the cooldown in force. ant-node only logs the return value, so scheduling and wire behaviour do not change; callers of the public method now get the remaining cooldown in force. The never-shortened test now checks the value too. The same debug line said the holder was passed over "for new keys", but the cooldown applies both when a key starts and when it moves on to another holder. It now says the holder is put last. ADR-0025: - the context adds the refusals WithAutonomi#252 leaves at holders it currently counts as silent, and says WithAutonomi#252's ADR defers a busy reply rather than naming it as the remaining piece; - the trade-off on holders that answer busy to everything says they are put behind every holder that is not cooling, on retries too, and names the `fetch_busy_reply_send_pool_full` counter in full; - the requester section states the cooldown as 1, 2, 4 and 8 times the jittered wait drawn for each busy answer, capped at 60 s, says that a later busy answer never shortens a running cooldown, and gives the table's eviction order when every holder in it is cooling; - the request-rate trade-off is given for any fetch timeout, not only the default 30 s; - the requester end-to-end test is described as filling that requester's own two-request share at the holder; - `fetch_busy_sources_tracked` is listed as a gauge, apart from the cumulative counters; - the positive consequence notes that the busy wait can be cut short by the attempt's own fetch timeout; - the rejected retry-rotation option states its cost as the mechanism (one request per round trip instead of one per fetch timeout) rather than an estimate; - the order section records that this change and WithAutonomi#252 conflict and have not been built or measured together yet; - the real-node measurements are rerun on upstream main a6e3388, and the local simulation, which is not published, is named only as design input rather than offered as validation.
dirvine
left a comment
There was a problem hiding this comment.
Provisional review — changes recommended
Reviewed WithAutonomi/ant-node@6abba5a79a7f04be40e11aa12c4a638148989ace against 8d25f0c311a964ba8f440fe4d0b9359b6294dfc8. All 11 changed files were inspected. This is a review of that standalone head, not a build of #253 combined with #252/current main.
Material concern: bound the trust-neutral Busy exemption
src/replication/mod.rs:9637–9672 treats every matching-key Busy as trust-neutral. SourceBusyTracker (src/replication/fetch_busy.rs:301–410) adds a bounded cooldown, but only changes preference: it still selects the first holder when every candidate is cooling, including a sole holder. There is no per-source/path exhaustion transition.
A holder can retain the data, answer storage audits, and answer replication fetches with correctly signed Busy indefinitely. Authentication proves who sent the answer, not that the sender is genuinely overloaded. Each fetch fails over after 5–10 seconds, but repeated non-service never reaches normal fetch accountability. Before this new response, a response timeout reaches ConnectionTimeout in the pinned saorsa-core 0.29.0 (network.rs:1300–1319); a valid NotFound reaches ApplicationFailure in ant-node. Thus this is a new neutral branch, not merely inherited silence.
I exercised the actual public tracker in a reviewer-only Rust integration probe. After 1,000 Busy answers across 5,000 seconds of simulated time, with no service, the same sole holder remained eligible on every selection. The probe passed and was removed from the checkout afterwards. This proves the scheduler has no exhaustion bound; it is not a live malicious-peer or audit experiment. The trust effect follows from the production Busy branch above.
The minimum safety change is a bounded peer/path-level allowance for neutral overload claims, with a defined accountability outcome after exhaustion and recovery tied to cooperative responses on the same path. A per-key retry limit or a longer cooldown alone does not cover repeated claims across keys. Add a regression for repeated valid Busy across multiple keys, alongside the existing honest-transient-overload test. ADR-0025 explicitly defers a service-audit solution; if indefinite neutrality is intentional, it needs an explicit maintainer security decision. I am not treating that trade-off as already accepted.
Publication and verification gates — separate from the concern above
- #252 is now merged. This standalone head uses
saorsa-core 0.29.0; current maina6e3388e79d0907ef678bcb6ec0fcaa12822e148uses0.29.1. The PR description requires integration with #252 and says that combined build has not been measured. GitHub currently reportsMERGEABLE, so I am not claiming a current merge conflict. Update against main and rerun the share/cooldown/retry and Busy E2E checks on the resulting head. No combined-build claim is made here. - No dev-testnet smoke was run by me. The PR classifies this as T3 and explicitly says dev-testnet evidence is still required before merge.
- Security Audit is red for four RustSec advisories in
rustls 0.23.45, fixed upstream in>=0.23.46: RUSTSEC-2026-0346, -0347, -0348 and -0349.Cargo.tomlandCargo.lockare unchanged by this PR; the frozen base and head have the identical lockfile blob. This is an inherited dependency/CI gate, not a new Busy regression. Exact failing job.
Checks I ran on the exact head
Runtime: macOS, rustc/cargo 1.96.0; --locked, test-utils, isolated Cargo target directory.
cargo test --locked --lib --features test-utils -- --test-threads=1: 1,215 passed. The separately run replication subset also passed, 669 tests; it is included in the library total, not additional coverage.cargo test --locked --test e2e --features test-utils fetch_busy_reply:: -- --nocapture --test-threads=1: 2 passed. These cover capability-gated refusal and the requester's neutral Busy handling.- An attempted additional filter,
fetch_busy_ab::fetch_busy_ab_global_pool_full, matched zero tests. It is excluded from the pass counts; it provides no extra global-pool evidence. - Opt-in
fetch_busy_ab_pool, isolated rerun: 40/40 chunks stored, 26 refusals observed, measured storage interval 10.252 s, zero measured trust loss at all five holders. The whole test took 52.45 s. An earlier run concurrent with other tests also stored 40/40, but its storage interval was 39.784 s (101.03 s for the whole test), with trust loss at all five holders. I do not attribute that difference to a code regression; it shows that the load measurement depends on the run conditions. - Reviewer-only repeated-Busy tracker probe: 1 passed, as described above.
cargo fmt --checkand exact base-to-headgit diff --check: passed. The tracked checkout is unchanged.- GitHub: 23 checks successful; 1 failed, the inherited Security Audit above. These are CI results, distinct from my local runs. The complete live-network/dev-testnet suite was not run.
Panel coverage, dissent and status
4/6 reviewed — partial panel, two of three named models. GLM 5.3/OpenRouter and local Qwen 3.8 each completed correctness/evidence and adversarial reviews. Both DS4 roles timed out in the 120-second model-lock queue; each received its one same-model narrowed retry and timed out again. No substitute model was counted.
GLM found no merge-blocking code defect and considered the documented cooldown policy acceptable. Qwen recommended changes, but its alleged GCRA fencepost/overflow, responder-versus-requester pool exhaustion and detached-send leak did not survive source/test checks: the fixed-time burst tests pass, duration multiplication saturates and is capped, the cited limits belong to responders, and Busy sends have owned RAII grants plus a two-second timeout. I have not repeated those as findings.
The persistent-neutrality concern above is my source- and probe-backed adjudication against the existing overload-accountability review criterion, not panel consensus. Focused GLM/Qwen reconciliation attempts also timed out in their model-lock queues. The material design disagreement therefore remains unresolved. Missing DS4 coverage alone would not prevent a formal verdict, but that unresolved disagreement does: this is a provisional COMMENT, not APPROVE or REQUEST_CHANGES. A human maintainer can decide the security trade-off and submit the formal review. No merge, branch update or deployment was performed.
A busy answer was trust-neutral without limit. A holder that kept the
data and answered its storage audits could refuse every fetch with Busy
for good at no cost: the requester's SourceBusyTracker only put it last
and never withheld it.
Each holder now gets FETCH_BUSY_NEUTRAL_ALLOWANCE, eight busy answers in
a row to a requester, counted across all keys, before it next serves
that requester. Past the allowance, each further busy answer is handled
as the silent refusal it replaces. The attempt holds its slot until its
fetch timeout runs out, never longer however late the answer came, and
the holder is then charged one unit-weight ApplicationFailure, which
saorsa-core scores the same as the ConnectionTimeout that silence
earned. The timeout counts from before any dial, while silence's counts
from after it, so after a cold dial the charge can come up to the dial's
time sooner. An attempt cancelled by shutdown first is not charged.
Only a served chunk restores the allowance. Time does not, and the
4096-entry table never forgets a record to make room: when it is full, a
holder it does not track yet is charged as for silence. Ten minutes
without a busy answer still restart the cooldown multiplier, but not the
count. The count lives in memory, so a requester restart starts every
holder afresh.
SourceBusyTracker::record_busy now returns BusyAnswer { cooling_for,
charged }, and busy_wake says when the attempt then lets go of its slot.
The admission summary line gains a fetch_busy_charged counter. New unit
tests cover:
- a late answer never holding the slot past the fetch timeout;
- a holder that never serves;
- answers that arrive together;
- serving between spells;
- recovery;
- time alone;
- a full table.
A new end-to-end test has a real holder refuse the requester's fetch
worker key after key until it is charged, as its attempt ends, and then
clears the holder's count once it serves. The test-utils hook
fetch_source_unserved_busy_for_test exposes that count. ADR-0025 records
the decision.
dirvine
left a comment
There was a problem hiding this comment.
Re-review — Busy accountability concern resolved; merge gates remain
Reviewed WithAutonomi/ant-node@a5490a9368693439e3743394bd001f6e628a747b against 8d25f0c311a964ba8f440fe4d0b9359b6294dfc8. This supersedes the Busy-accountability finding in my previous review, not its historical test results. I found no new confirmed source defect in the revised accountability path. This is not merge or release approval.
What is now fixed
src/replication/fetch_busy.rs:68–87, 410–453: a source gets eight neutral Busy answers across keys since last serving this requester. The ninth and every later answer are charged. Cooldown expiry or silence does not clearunserved; it saturates rather than wrapping. A full 4,096-source table does not evict debt: an untracked source then gets no neutral allowance.src/replication/mod.rs:9648–9703andfetch_busy.rs:167–184: an over-budget answer holds its slot until the attempt's fetch deadline, then awaits oneApplicationFailure(REPLICATION_TRUST_WEIGHT)event, with weight1.0. This is not an immediate penalty per reply. The deadline is measured from the attempt start; after a cold dial it can precede the old transport response-timeout deadline by the dial duration, as the source documents.src/replication/mod.rs:9536–9580: the source's record clears only after the response supplies the expected key and passes size and content-address checks. A locally rejected store or lapsed responsibility after receiving valid bytes does not make that remote service non-cooperation. Already-held and invalid-data exits do not manufacture service. Local shutdown can cancel an uncharged pending fetch; the tracked task otherwise retains its charge when the outer worker awaiter is dropped.
The new real-node regression a_holder_that_keeps_answering_busy_is_charged_past_its_allowance passed: it spends the allowance across different keys, checks no trust loss within it, checks no early charge on the next attempt, then observes a charge and verifies that later valid service clears the record. The previous always-Busy concern is therefore addressed by code and an exercised regression, not merely by a cooldown.
Verified locally at this exact head
macOS arm64, rustc 1.96.0, isolated CARGO_TARGET_DIR; no tracked review edits:
cargo test --locked --lib --features test-utils -- --test-threads=1: 1,221 passed, including the 22 Busy unit tests (not 22 additional unique tests).cargo test --locked --test e2e --features test-utils fetch_busy_reply:: -- --nocapture --test-threads=1: 3 passed.- Opt-in pool regression (
ANT_FETCH_BUSY_AB=1, exactfetch_busy_ab::fetch_busy_ab_pool): 40/40 chunks stored, 20 requesters, 26 full-pool refusals; requester-charge counts[0,0,0,0,0], measured trust loss[0.0,0.0,0.0,0.0,0.0]across five holders. This is a local real-node test, not dev-testnet evidence and not a fresh main-versus-PR comparison. - Default
cargo check --locked --liband--no-default-featuresboth passed. This resolves the first-pass concern about a test-only gate leaking into production compilation. Format and diff-whitespace checks passed. - Local
cargo clippy --locked --lib --all-features -- -D warningsfailed on the twoDuration::from_secs(60/600)unit-style lints. The same failures reproduce at the previous PR head6abba5awith this compiler; this proves they are not introduced by the latest fix, not that they were present at the PR base. CI Clippy is green. A command-line-Acontrol also failed; I do not count it as a passing check.
Gates before merge
- Integrate with #252 and rerun together. Live #252 is OPEN, head
31f028a2b84013841504783b32ae750842f9c83d. Agit merge-treecheck against this #253 head reports conflicts insrc/replication/mod.rsanddocs/adr/README.md. The PR itself requires merging after #252 and says the pair has not been built/measured together. Resolve/rebase, then rerun the combined build, Busy regressions and pool measurements. Maina6e3388e79d0907ef678bcb6ec0fcaa12822e148text-merges cleanly, but I did not build that combined tree; a clean merge is not integration-test evidence. - Supply the required T3 dev-testnet evidence on that integrated build. The PR explicitly says it has not been run. The local loopback harness and the green CI “WebRTC Direct five-node devnet” do not establish that requirement. The ADR is still Proposed; its final acceptance remains a human decision.
- Clear or explicitly resolve the failing Security Audit under repository policy. At
2026-10-20T07:54:54.310028+00:00, CI shows 21 successful, 1 failed, 2 pending. Security Audit reports seven vulnerabilities: four inrustls 0.23.45(upgrade recommendation>=0.23.46) and three inzip 2.4.2(>=9.0.0).Cargo.lockis byte-identical between this head and the frozen PR base, so these are dependency-gate failures, not dependencies changed by this PR. This is not an assessment that every advisory is remotely exploitable in ant-node. Do not describe CI as green while that job fails or other jobs remain pending.
Panel, dissent and limitations
5/6 scoped role reviews completed; 3/3 named models. Partial panel. Missing: deepseek-v4-flash adversarial-risks (output token limit (8192 tokens); no complete verdict).
z-ai/glm-5.3/ correctness-evidence: APPROVE (narrowed fix scope).qwen38-27b/ correctness-evidence: APPROVE (narrowed fix scope).deepseek-v4-flash/ correctness-evidence: APPROVE (narrowed fix scope).z-ai/glm-5.3/ adversarial-risks: APPROVE (narrowed fix scope).qwen38-27b/ adversarial-risks: APPROVE (narrowed fix scope).deepseek-v4-flash/ adversarial-risks: unavailable after the single same-model retry; output token limit (8192 tokens); no complete verdict.
Input-error disclosure: I incorrectly described the neutral allowance as three in the first and GLM/Qwen narrowed briefs, and named a nonexistent fourth-answer regression. The committed constant and the actual test use eight. Qwen's first-pass adversarial objection correctly identified that discrepancy: it was an error in my brief, not a defect in the PR. Some narrowed responses then repeated my error. I reject their three/fourth threshold statements as evidence; the eight/ninth result above comes from frozen-source inspection and the actual passing E2E. The corrected DS4 retry includes the literal constant and the correction. No additional GLM/Qwen retry is used to obtain agreement.
GLM and Qwen's completed narrowed responses accept the state persistence, fail-closed saturation and delayed charge. First-pass feature-gating uncertainty was resolved by the actual cfg source and two production-feature builds. Retained design watch-outs: the table is requester-memory-only (restart renews each source's allowance); one valid service renews it; saturation conservatively charges newcomers. These are disclosed bounds/trade-offs, not a demonstrated remaining always-Busy escape. Narrowed retry approval covers the accountability fix and connected lifecycle, not a fresh line-by-line review of unchanged trailer/limiter/scheduler code. Missing seats are coverage limits, never code defects. No unqualified full-team consensus is claimed.
Next: finish #252 integration, rerun the gates on the resulting head, provide dev-testnet evidence and resolve the audit/CI status before merge. No merge was performed.
dirvine
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — required integration and T3 validation are missing
Reviewed WithAutonomi/ant-node@a5490a9368693439e3743394bd001f6e628a747b against 8d25f0c311a964ba8f440fe4d0b9359b6294dfc8. This is the formal verdict requested by Anselme in the existing review thread. The source head is unchanged from my verified re-review.
The previous always-Busy finding is closed. Eight neutral Busy answers are allowed across keys; the ninth and subsequent answers incur a delayed unit-weight charge. Time, cooldown and table saturation do not replenish the allowance. I found no new confirmed defect in that standalone accountability path. The changes requested below concern the intended integrated change and its mandatory validation, not a claim that the fixed logic still allows indefinite neutrality.
Required changes before approval
-
Complete the #252 integration and verify the resulting source tree. Location:
docs/adr/ADR-0025-fetch-busy-reply.md:135–141, with the integration insrc/replication/mod.rsand #252's source selection. The ADR and PR description explicitly require #252 first and say the pair has not been built or measured together. #252 is currently OPEN at31f028a2b84013841504783b32ae750842f9c83d. The recorded merge-tree check of these exact heads conflicts insrc/replication/mod.rsanddocs/adr/README.md. Without the per-source share, requesters can still over-share and refusals beyond the reply limits remain silent, as the ADR explains. Minimum change: after #252 lands, rebase/integrate it, resolve the source-choice/cooldown interaction, and rerun the combined build, share/cooldown/retry regressions, Busy E2E tests and real-node pool measurements. Attach results for that exact resulting commit, not separate results for the two PRs. -
Provide the required T3 dev-testnet evidence for that integrated commit. Location: the PR description's
Risk tierandTest evidence / Dev testnetsections. It explicitly says this evidence is required before merge and has not been run. This changes the wire protocol and requester scheduling/trust behaviour. Local loopback tests and CI's WebRTC Direct five-node devnet do not prove the required integrated fetch behaviour. Minimum change: run the approved dev-testnet validation on the rebased build; record the commit/build, configuration, run ID, commands, results and logs in the PR. Include the Busy/share interaction and retained mixed-version compatibility. My review does not authorise deployment.
Current checks — separate dependency gate
At 2026-10-20T08:23:49.786027+00:00, CI has 23 successful, 1 failed, 1 skipped, and no pending checks. Security Audit still fails on the unchanged rustls/zip lockfile dependencies. Resolve it or obtain its disposition under repository policy before merge; I am not presenting inherited advisories as a new defect introduced by this PR. My exact-head runs passed 1,221 library tests, 3 Busy E2E tests, both default/no-default production checks and the 40/40-chunk pool test. The previously disclosed local Duration-unit Clippy failures remain distinct from the green CI Clippy job.
Panel and limitations
5/6 scoped role reviews completed — partial panel. GLM 5.3, Qwen 3.8 and DS4 completed correctness reviews; GLM and Qwen also completed adversarial reviews. DS4's adversarial output hit its 8,192-token limit after its single same-model retry. No further retry or substitute vote was used. The five completed narrowed reviews accept the implementation within their scope; this formal request concerns the still-unmet integration and T3 validation requirements, not a claim that those five models rejected the standalone fix. No full-team consensus is claimed.
The earlier review brief incorrectly said three neutral answers; the committed constant is eight. As disclosed in the linked re-review, GLM/Qwen's resulting three/fourth-answer statements are excluded as evidence. The eight/ninth result is established by exact source, the passing regression and DS4's corrected correctness review. Narrowed reviews do not constitute a fresh full-production review of unchanged wire/limiter/scheduler hunks.
Next: complete the integration and required validation, then request re-review on the resulting head. No merge, branch change or deployment was performed.
…on share A fetch responder admits two outstanding requests per requesting peer and drops later arrivals without replying while that share is full. The fetch worker ran one fetch per hardware thread and sent each one to the first verified holder of its key, and the nearest-first queue often groups keys that share holders. When more than two fetch slots were free and several queued keys shared a holder, the worker sent more than that holder's share. Each resulting drop cost the full 30 s fetch timeout, a connection-timeout trust event against a holder that did nothing wrong, and a retry that ignored how busy the next holder was. The fetch worker now keeps at most FETCH_MAX_IN_FLIGHT_PER_SOURCE (the responder's share) of its fetches in flight to any one source that is answering: - Each free slot goes to the nearest queued key that can start now, on the first untried source in its preference order that is answering and below the limit, or else on its first silent source. - A key whose untried sources are all answering and full is parked: it leaves the fetch heap, is indexed exactly under each of those sources, and starts again as soon as one frees a share or falls silent, competing with the heap on distance. - A failed fetch moves on by the same rule. If every remaining source is answering and full, it is parked instead of being sent to a full holder, keeping its tried sources and its verification retry reservation. With no untried source left, or a full fetch queue, it returns to verification as before. - The sources a queued or in-flight key is associated with, tried ones included, are capped at close_group_size + paid_list_close_group_size + MAX_HINT_SOURCES_PER_KEY from the running configuration. A re-advertisement merges into such a key's verification retry entry under MAX_HINT_SOURCES_PER_KEY, as into a pending entry, instead of without a bound. A holder that stops answering has no share to keep within, and holding it to two requests per fetch timeout would drain the keys only it holds four times more slowly than before. Each attempt now records whether its source answered. Requests and answers take places in one process-wide sequence, so they are ordered exactly whatever order their outcomes are applied in. An answer reaches the scheduler as soon as it arrives, before its chunk is checked and stored, so a timeout it outdates cannot mark its source while that chunk is still being written. A source is silent while a request sent to it after its latest answer has gone unanswered, and a silent source is fetched from past the limit, as before this change, until it answers again, or until no newer unanswered request to it has been seen for two fetch timeouts (at least one second), after which it is probed under the limit. The table is bounded at 8,192 sources and drops only entries that hold no mark and guard no in-flight fetch; if that frees nothing, untracked sources count as silent for one mark lifetime, which again is the behaviour before this change. A "replication fetch dispatch summary" line every 5 minutes and at shutdown reports parked keys, index size, oldest parked age, queued and in-flight fetches, sources at the limit, parks, attempts by outcome, stored bytes, attempt latency, cancelled attempts, fill-pass lock wait and hold, parked-index maintenance time, and the silent-source counts and events. The limit removes over-share bursts but cannot rule out every refusal: a request this node has already timed out on may still hold its share at the responder until a worker sheds it. Responder admission, the fetch timeout, fetch concurrency and the wire protocol are unchanged. ADR-0024 records the decision, a local risk simulation and a scale benchmark at the fetch queue's hard bound, which runs when ANT_FETCH_SCALE_BENCH names a sources-per-key count. Test-only hooks, compiled only under cfg(test) or the test-utils feature and scoped to one engine: the fetch concurrency, a gate that holds the engine's admitted fetch requests before serving them and that engine shutdown releases, and a fetch-pipeline snapshot. With concurrency eight and twelve keys directed to one holder, the requester records at most two in-flight fetches to it. With a 5 s fetch timeout and sixteen keys directed to a holder that answers nothing, the requester sends past the share once the first requests time out and drains the keys in three timeouts.
…ours Clippy's duration_suboptimal_units lint, as of Rust 1.96, flags Duration::from_secs(60), (600) and (3600). Build those durations with from_mins and from_hours instead, as the crate already does elsewhere. Every value is unchanged.
Bring in WithAutonomi#252 (447c000, on main b0e29c3) so that this branch is the combined build WithAutonomi#252 and WithAutonomi#253 will run as. The busy tracker now feeds WithAutonomi#252's scheduler. ReplicationQueues holds the same SourceBusyTracker that the fetches record busy answers in. Its dispatch and its retry put a holder that recently answered busy after every other free holder, and still before a silent one. The tracker's own source choice is removed, and retry_fetch is back to main's form. A busy answer is a response, so it already counts as an answer for WithAutonomi#252's silent-source tracking. A busy wait keeps its attempt in flight, so it counts against the holder's per-source share. Both branches added the same fetch serve gate hooks; one copy is kept.
With WithAutonomi#252's per-source scheduler, one requester no longer sends more than a holder's two-request share, so the over-share measurement can no longer produce a busy reply. Keep the measurement of the refusals that WithAutonomi#252 leaves, a holder whose admission pool is full of other peers' requests, and say so in the file's description.
…hat refusals answer busy WithAutonomi#252's a_holder_that_stops_answering_is_fetched_from_past_its_share closes the holder's fetch serve gate so that the holder answers nothing. With busy replies, that holder still answers busy to the requests it refuses. It is then not silent, and WithAutonomi#252's per-source cap rightly applies to it. Add a test-only switch, turn_off_fetch_busy_replies_for_test, that makes a holder refuse silently. Use it there, so that the test again models a holder that has stopped answering. Also correct the A/B file's description of the test hooks it needs.
…ld measurements Record how the busy reply works with WithAutonomi#252's per-source scheduler: a cooling holder goes after every other free holder, a busy answer counts as an answer, and a busy wait counts against the holder's share. Replace the measurements with runs on the combined build and on WithAutonomi#252 alone. Keep the earlier runs without WithAutonomi#252 as history.
… runs Quote the four pool runs made on the combined build after the last code change, and every earlier run in place of rounded ranges.
Under WithAutonomi#252's per-source share, a holder that just answered busy is passed over only for another free holder of the same key.
…on share A fetch responder admits two outstanding requests per requesting peer and drops later arrivals without replying while that share is full. The fetch worker ran one fetch per hardware thread and sent each one to the first verified holder of its key, and the nearest-first queue often groups keys that share holders. When more than two fetch slots were free and several queued keys shared a holder, the worker sent more than that holder's share. Each resulting drop cost the full 30 s fetch timeout, a connection-timeout trust event against a holder that did nothing wrong, and a retry that ignored how busy the next holder was. The fetch worker now keeps at most FETCH_MAX_IN_FLIGHT_PER_SOURCE (the responder's share) of its fetches in flight to any one source that is answering: - Each free slot goes to the nearest queued key that can start now, on the first untried source in its preference order that is answering and below the limit, or else on its first silent source. - A key whose untried sources are all answering and full is parked: it leaves the fetch heap, is indexed exactly under each of those sources, and starts again as soon as one frees a share or falls silent, competing with the heap on distance. - A failed fetch moves on by the same rule. If every remaining source is answering and full, it is parked instead of being sent to a full holder, keeping its tried sources and its verification retry reservation. With no untried source left, or a full fetch queue, it returns to verification as before. - The sources a queued or in-flight key is associated with, tried ones included, are capped at close_group_size + paid_list_close_group_size + MAX_HINT_SOURCES_PER_KEY from the running configuration. A re-advertisement merges into such a key's verification retry entry under MAX_HINT_SOURCES_PER_KEY, as into a pending entry, instead of without a bound. A holder that stops answering has no share to keep within, and holding it to two requests per fetch timeout would drain the keys only it holds four times more slowly than before. Each attempt now records whether its source answered. Requests and answers take places in one process-wide sequence, so they are ordered exactly whatever order their outcomes are applied in. An answer reaches the scheduler as soon as it arrives, before its chunk is checked and stored, so a timeout it outdates cannot mark its source while that chunk is still being written. A source is silent while a request sent to it after its latest answer has gone unanswered, and a silent source is fetched from past the limit, as before this change, until it answers again, or until no newer unanswered request to it has been seen for two fetch timeouts (at least one second), after which it is probed under the limit. The table is bounded at 8,192 sources and drops only entries that hold no mark and guard no in-flight fetch; if that frees nothing, untracked sources count as silent for one mark lifetime, which again is the behaviour before this change. A "replication fetch dispatch summary" line every 5 minutes and at shutdown reports parked keys, index size, oldest parked age, queued and in-flight fetches, sources at the limit, parks, attempts by outcome, stored bytes, attempt latency, cancelled attempts, fill-pass lock wait and hold, parked-index maintenance time, and the silent-source counts and events. The limit removes over-share bursts but cannot rule out every refusal: a request this node has already timed out on may still hold its share at the responder until a worker sheds it. Responder admission, the fetch timeout, fetch concurrency and the wire protocol are unchanged. ADR-0024 records the decision, a local risk simulation and a scale benchmark at the fetch queue's hard bound, which runs when ANT_FETCH_SCALE_BENCH names a sources-per-key count. Test-only hooks, compiled only under cfg(test) or the test-utils feature and scoped to one engine: the fetch concurrency, a gate that holds the engine's admitted fetch requests before serving them and that engine shutdown releases, and a fetch-pipeline snapshot. With concurrency eight and twelve keys directed to one holder, the requester records at most two in-flight fetches to it. With a 5 s fetch timeout and sixteen keys directed to a holder that answers nothing, the requester sends past the share once the first requests time out and drains the keys in three timeouts.
WithAutonomi#252 was rebased onto main 2598bcf (WithAutonomi#250) as 33ec231, which is 447c000 plus exactly WithAutonomi#250's changes. Bring it in; the only real conflict is the ADR index, which now lists ADR-0023, ADR-0024 and ADR-0025. The result is the previous head plus WithAutonomi#250's changes.
…on share A fetch responder admits two outstanding requests per requesting peer and drops later arrivals without replying while that share is full. The fetch worker ran one fetch per hardware thread and sent each one to the first verified holder of its key, and the nearest-first queue often groups keys that share holders. When more than two fetch slots were free and several queued keys shared a holder, the worker sent more than that holder's share. Each resulting drop cost the full 30 s fetch timeout, a connection-timeout trust event against a holder that did nothing wrong, and a retry that ignored how busy the next holder was. The fetch worker now keeps at most FETCH_MAX_IN_FLIGHT_PER_SOURCE (the responder's share) of its fetches in flight to any one source that is answering: - Each free slot goes to the nearest queued key that can start now, on the first untried source in its preference order that is answering and below the limit, or else on its first silent source. - A key whose untried sources are all answering and full is parked: it leaves the fetch heap, is indexed exactly under each of those sources, and starts again as soon as one frees a share or falls silent, competing with the heap on distance. - A failed fetch moves on by the same rule. If every remaining source is answering and full, it is parked instead of being sent to a full holder, keeping its tried sources and its verification retry reservation. With no untried source left, or a full fetch queue, it returns to verification as before. - The sources a queued or in-flight key is associated with, tried ones included, are capped at close_group_size + paid_list_close_group_size + MAX_HINT_SOURCES_PER_KEY from the running configuration. A re-advertisement merges into such a key's verification retry entry under MAX_HINT_SOURCES_PER_KEY, as into a pending entry, instead of without a bound. A holder that stops answering has no share to keep within, and holding it to two requests per fetch timeout would drain the keys only it holds four times more slowly than before. Each attempt now records whether its source answered. Requests and answers take places in one process-wide sequence, so they are ordered exactly whatever order their outcomes are applied in. An answer reaches the scheduler as soon as it arrives, before its chunk is checked and stored, so a timeout it outdates cannot mark its source while that chunk is still being written. A source is silent while a request sent to it after its latest answer has gone unanswered, and a silent source is fetched from past the limit, as before this change, until it answers again, or until no newer unanswered request to it has been seen for two fetch timeouts (at least one second), after which it is probed under the limit. The table is bounded at 8,192 sources and drops only entries that hold no mark and guard no in-flight fetch; if that frees nothing, untracked sources count as silent for one mark lifetime, which again is the behaviour before this change. A "replication fetch dispatch summary" line every 5 minutes and at shutdown reports parked keys, index size, oldest parked age, queued and in-flight fetches, sources at the limit, parks, attempts by outcome, stored bytes, attempt latency, cancelled attempts, fill-pass lock wait and hold, parked-index maintenance time, and the silent-source counts and events. The limit removes over-share bursts but cannot rule out every refusal: a request this node has already timed out on may still hold its share at the responder until a worker sheds it. Responder admission, the fetch timeout, fetch concurrency and the wire protocol are unchanged. ADR-0024 records the decision, a local risk simulation and a scale benchmark at the fetch queue's hard bound, which runs when ANT_FETCH_SCALE_BENCH names a sources-per-key count. Test-only hooks, compiled only under cfg(test) or the test-utils feature and scoped to one engine: the fetch concurrency, a gate that holds the engine's admitted fetch requests before serving them and that engine shutdown releases, and a fetch-pipeline snapshot. With concurrency eight and twelve keys directed to one holder, the requester records at most two in-flight fetches to it. With a 5 s fetch timeout and sixteen keys directed to a holder that answers nothing, the requester sends past the share once the first requests time out and drains the keys in three timeouts.
WithAutonomi#252 was rebased onto main a5bccf0 (WithAutonomi#258 and the 0.23.1 release) as 81325f4, which is 33ec231 plus exactly those changes to Cargo.toml and Cargo.lock. Bring it in. The conflicts come only from the same WithAutonomi#252 patch landing twice, so this branch's side is kept. The result is the previous head plus main's Cargo.toml and Cargo.lock changes.
MaidSafe-Hermes
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — required T3 dev-testnet evidence is still missing
Reviewed WithAutonomi/ant-node@b0cef9268c046fee546cd8f5a53a0fa0649e1b43 against base a5bccf07e35d5f9d03ba7c5329a90b05525a47b3. The branch now contains #252 at 81325f4077ff899a24b05ab1a1b69fae37f56735; its current PR head is that same commit, although #252 remains open. I found no new confirmed source defect in the Busy reply implementation or its integration with the per-source scheduler.
Required before approval
- Run and record the required T3 dev-testnet validation on this exact combined head. The PR description says this has not been run and requires one join-wave run of the combined #252+#253 head. The successful CI
WebRTC Direct five-node devnetcheck is useful CI evidence, but it is not the described join-wave dev-testnet run. Add the exact commit, run ID, configuration, commands, results and logs, including the Busy/share interaction and mixed-version compatibility. If the test is run on a newer commit, re-review that head.
This is a missing validation gate, not a claim of a code defect. The previous Busy-accountability finding is resolved: the source retains the eight-answer neutral allowance, charges later answers after the fetch deadline, and does not replenish the allowance on time or tracker-table pressure. The request is limited to the explicitly required T3 evidence.
Checks and review coverage
- GitHub checks at this head: 21 SUCCESS, including Security Audit and WebRTC Direct five-node devnet.
cargo fmt --all -- --check: passed locally.- A local focused Cargo test attempt did not finish within the tool's time limit; it is not counted as a pass. The exact-head GitHub Test (ubuntu-latest), Test (macos-latest), and Test (windows-latest) checks are successful.
- 2/2 review seats returned substantive opinions across two scoped evidence bundles:
sol-1(gpt-6.1-sol, xhigh) andglm-1(z-ai/glm-5.3). GLM approved the code within scope. Sol returnedUNCERTAINwhere the first bundle lacked integration context; its remaining specific evidence gaps were checked against the frozen source. I found no confirmed defect. This is not panel consensus.
No merge, branch update or deployment was performed.
A node that refuses a fetch at admission now tells the requester
Busyat once, when the request asked for it and the reply fits within the responder's limits, instead of staying silent. That silence cost every refused requester a 30 s wait and charged the refusing holder a connection-timeout trust event it had not earned.The problem. A fetch responder admits two outstanding requests per requesting peer and twelve overall. It refuses the rest at admission (
Fetch response dropped at admission) and sends nothing back. Then:ConnectionTimeoutagainst the holder, which only did what its limits say;#252's description reports, from the responders' cumulative admission counters, that 36.6% of inbound fetch requests on 0.21.0 and 44.3% on 0.22.0 were refused this way, and that the WARN line counted 3.38M refusals in the 25-hour window containing the 2026-10-11 join wave of 177 new nodes. #252 keeps a requester within each answering holder's share, which removes most per-peer over-share. Refusals remain for a busy reply to answer: a share still held by requests the requester already gave up on (which #252's description leaves to a busy reply), a twelve-slot pool full of other peers' requests, and a holder #252 currently counts as silent, which it may send requests past the share until an answer clears the mark or the mark expires and the holder is probed under the share again. Older requesters stay on the old silence until they upgrade.
The change.
Wire. A requester that understands busy replies appends a 5-byte capability trailer (
antfand a flags byte) after the encodedFetchRequest.FetchRequest's own layout is unchanged. Older nodes decode with postcard'sfrom_bytes, which ignores trailing bytes, so they read the key and keep refusing silently.FetchResponse::Busy { key }is appended after the existing variants, whose discriminants are unchanged. It is sent only in answer to a request carrying the trailer, so an older requester never receives it.Responder. Every message is signed (about 5 KiB with the ML-DSA-65 key and signature), so busy replies are bounded:
Anything outside those bounds is the old silence. The refusal WARN line keeps its text and counters and gains
capabilityandreplyfields.Requester. The fetch worker's requests ask for busy replies. A
Busyfor any other key than the one requested is charged as malformed, like every key mismatch. ABusyfor the requested key carries no trust event while its holder is within its neutral allowance: eight busy answers in a row to this requester, counted across all keys, since the holder last served it. Past the allowance, each further busy answer is handled as the silent refusal it replaces: the attempt holds its slot until its fetch timeout runs out, never longer however late the answer came, and then the holder is charged one unit-weightApplicationFailure, which saorsa-core scores the same as theConnectionTimeoutsilence earned. Only serving this requester once restores the allowance; time does not. Within the allowance, the attempt waits 5-10 s with jitter, never past its own fetch timeout, still holding its fetch slot, then ends as a source failure. So the queue moves on to the next holder exactly as after a timeout, and a busy answer cannot become a fast retry loop. While its cooldown runs, fix(replication): keep fetches within each answering holder's admission share #252's per-source scheduler puts the busy holder after every other free holder when starting or retrying keys, and still before a silent one, so a holder is never withheld. The cooldown is 1, 2, 4, then 8 times the full jittered wait drawn for that answer, even when the attempt's deadline cut the sleep short, capped at 60 s, and a later busy answer never shortens a running cooldown. Serving once resets the multiplier, and so do 10 minutes without a busy answer, which leave the allowance count as it was. The table holds 4096 holders, and a holder leaves it only by serving. When it is full, a holder it does not track yet is charged for each busy answer, as for silence, and is not put last.Built on #252. This branch carries #252's commit (81325f4, #252's head on main a5bccf0, and its earlier forms 33ec231 and 447c000) and merges after it. Until #252 merges, the diff below includes #252; this PR's own changes are its commits other than those three. In the combined build:
What it does not do.
Linear issue
Closes V2-1514
Risk tier
Compatibility
FetchResponse::Busyvariant, appended, and an optional 5-byte capability trailer after aFetchRequestmessage. Older nodes ignore the trailer and are never sentBusy. Pinned by tests that decode with mirrors of the previous types.FetchRequestgains a publicbusy_replyfield (not serialized),FetchResponsegains a variant, and there is a new publicreplication::fetch_busymodule. Also new and public:protocol::{FETCH_CAPABILITY_MAGIC, FETCH_CAPABILITY_BUSY_REPLY}. Additive test-only hooks undertest-utils, beyond fix(replication): keep fetches within each answering holder's admission share #252's:ReplicationEngine::{fetch_source_cooling_for_test, fetch_source_last_busy_for_test, fetch_source_unserved_busy_for_test, fetch_admitted_for_test, fetch_pool_full_refusals_for_test, turn_off_fetch_busy_replies_for_test}.Semver impact
Test evidence
The combined build is #252 with this PR on top. Its current head, b0cef92, is #252 at 81325f4 on main a5bccf0 (#258 and the 0.23.1 release included). It is the previous head 1ba41d8 (#252 at 33ec231 on main 2598bcf) plus only main's changes to
Cargo.tomlandCargo.lock, which move saorsa-core to 0.29.2, ant-protocol to 5.0.1 and saorsa-transport to 0.38.2 from crates.io, andrustls-webpkiandwebpki-root-certsto fixed versions. The gates below and three real-node runs were made on b0cef92, and four more real-node runs on 1ba41d8. The other real-node runs in the table were made on 20eddd0, the same code on #252 at 447c000 and main b0e29c3, before #250. The table also has control runs of #252 alone, and the paragraph after it earlier runs without #252.Dev testnet: not run yet. #252 (T2) and this PR (T3) both need it before merge; one join-wave run of this head covers both.
Real nodes.
tests/e2e/fetch_busy_ab.rsis opt-in:ANT_FETCH_BUSY_AB=1 cargo test --test e2e --features test-utils fetch_busy_ab -- --nocapture --test-threads=1. 20 requesters, each within its two-request share; the first of five holders keeps its 12-slot pool full for 2 s, the refusal case left after #252 (28 refusals in every run).One more run of #252 alone stopped before measuring, at the test's check that every key was newly queued (38 of 40). Every measured run fetched every chunk.
Earlier, without #252, on main a6e3388: the same case took 90.3 s in one run on that main, against 9.4 s to 10.5 s in four runs, no holder charged, for this PR alone. A single requester over-asking three holders, the case #252 removes, took 90.2 s, 90.2 s and 90.3 s on that main, and 35.4 s to 39.2 s in six runs for this PR alone; that measurement left the file once #252 made the case impossible.
Raw output
End to end (
tests/e2e/fetch_busy_reply.rs), deterministic through a test-only fetch serve gate, the same one #252 adds:Busyat once, and one that does not ask is refused silently;Unit tests on top of #252 (21 new in
fetch_busy, 9 inprotocol, 2 inmod, 2 inscheduling):Busy;Simulation. A local, unpublished discrete-event model of the 177-node join wave and 24 edge cases informed the design, including the 5-10 s wait. Its source and raw output are not part of this PR, so its results are not offered as review evidence.
Gates run locally, on b0cef92:
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warnings, on stable and on Rust 1.99 (CI's toolchain), andcargo +1.96.1 clippy --locked --lib --all-features -- -D warningscargo clippy -- -D warningsRUSTDOCFLAGS="--deny=warnings" cargo doc --no-depscargo test --lib --features test-utils: 1271 passedRUSTFLAGS="-D warnings" cargo check --lib --no-default-featuresandRUSTFLAGS="-D warnings" cargo test --lib --no-default-features: 1226 passedcargo test --test e2e --features test-utils -- --test-threads=1: 125 passed, plus the threelive_testnettests that are ignored on mainpoc_d1_bounded_queues,poc_bootstrap_stall)scripts/adr-governance.pyagainst maincargo audit: no vulnerabilities; its six allowed warnings all come from main's lockfile, which this PR does not touchReview. Codex-assisted:
design: three rounds;
code: five rounds to "No findings", then a fresh full review on 2026-10-16, also "No findings". Its review of the second commit found that a debug line could report a shorter cooldown than the one in force; that is fixed in the same commit, and the commit was then reviewed again with "No findings";
the neutral allowance (2026-10-20): nine rounds to "No findings", plus five reviews with no history, the last two with "No findings". Fixed along the way:
Two limits were kept and documented: the count lives in memory, and after a cold dial the charge can come up to the dial's time sooner than silence's would. The last two of the reviews with no history treated both as out of scope;
building on fix(replication): keep fetches within each answering holder's admission share #252 (2026-10-20): three rounds to "No findings". Fixed along the way: the over-share A/B measurement, which fix(replication): keep fetches within each answering holder's admission share #252 made unable to produce a busy reply, was removed, and the A/B file's description of its test hooks was corrected;
this description and the ADR: reviewed separately until no findings remained.
New dependency
none
ADR
https://github.com/grumbach/ant-node/blob/fetch-busy-reply/docs/adr/ADR-0025-fetch-busy-reply.md
Mitigation / rollback
Ship a release without it; capability is negotiated per request, so nodes on either side of a mixed fleet keep working.