Repository navigation
feat(read): spread hot-record reads across the close group, and back off briefly from nodes that decline - #225
mickvandijke wants to merge 4 commits into
Conversation
…off briefly from nodes that decline Every client read a record from its nearest holder first, so a popular record loaded one node while the rest of its close group idled. Both read paths now choose uniformly among the close group as currently known (the nearest CLOSE_GROUP_SIZE candidates), then fall back to nearest-first. The hedge, early-read allowance and absence rules are unchanged. The engine takes the choice as a parameter so tests can pin nearest-first order. A browser dial that a node declined used to cost a 10-second timeout and a 30-minute dial-failure cooldown, so full nodes vanished from lookups exactly when demand peaked. A failed fresh dial is now classed Refused when the node answered it: the association failed without connecting within 3 s of ICE checks starting (a transport refusal, sent by saorsa-transport's capacity refusal), or the node closed the channel or association after ICE and DTLS completed. Refused endpoints are suppressed for 15 s; every other failure keeps the 30-minute cooldown. The channel-open wait now ends as soon as the browser reports the association dead. ant-node ADR-0023. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oonest expiry Review follow-ups: - A transport refusal now also requires that ICE never connected. A DTLS failure after ICE connected means a different node answered at the endpoint, and keeps the 30-minute cooldown. (Chromium marks the association failed while its legacy ICE state still reads "checking", so the check is "never connected", not "reads failed".) - After ICE and DTLS, only a close by the node counts as a refusal. A lane this client closed itself, for malformed ingress, failed its inbox with its own reason first and keeps the full cooldown. - Early reads apply the may-hold eligibility rule before the random choice, so an ineligible pick can no longer block a nearer eligible hint. - A full dial-failure cache evicts whichever record expires first, so a 15 s refusal never pushes out a 30-minute failure. - Less timing-sensitive cooldown unit test; the WASM regression now also checks that a DTLS failure keeps the full cooldown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r than the record absent A browser skips a node that declined a session for 15 s. A read that began while every holder it could find was skipped retried once after 1 s and then reported the record not found. On the ADR-0023 testnet every failed download ended this way. The read engine now asks the adapter which peers it is skipping for a refusal. While one of them may hold the record (fewer than a close group of nearer peers answered NotFound, and it did not answer itself), a round that finds nothing is not evidence of absence. The read waits until the first such refusal ends and tries again, up to three times. It then fails with "the nodes that may hold chunk ... are busy" instead of returning not found. A refusal that ended during the round counts as ready, so a holder skipped by discovery is never mistaken for an absent record. Native transports report no refusals, so native reads are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dirvine
left a comment
There was a problem hiding this comment.
Hermes cross-repository review — provisional COMMENT
Reviewed commit: 4717922be49cfb373cb9c6b50f9f5788c7b62c03. The local checkout matches it and is clean. This review covers ant-client #225 alongside saorsa-transport #183 and ant-node #250.
Verdict: I have not confirmed a new blocking code defect in this diff. This is not an approval: the required review panel is incomplete, and Security Audit is still failing.
Code and contract checks
read.rsshuffles the close group using the supplied uniform choice. It keeps peers outside that group in distance order. The progressive read still bounds concurrent GETs and preserves the final candidate-set absence rules.- Refusing possible holders get a bounded retry path: at most three refusal waits, then a busy error instead of an absence result. The 45 seconds described in the code is the refusal-wait allowance, not a total read deadline including discovery and RPCs.
- The browser separates 15-second refusal suppression from the long failure cooldown. JavaScript does not receive the STUN 486 code directly: the code uses association/ICE state and a three-second timing window. Treat browser-version changes as a regression risk, not proof that every quick failure is a capacity refusal.
Evidence and remaining gates
- Independently executed at this head:
cargo test --locked -p ant-core --lib: 819 passed, 0 failed. - Built the browser WASM with
browser-wasm,test-utils, then ran the generated WASM under Node with the repository's RTC test setup: 211 passed, 0 failed. This is not an independently run Chromium/WebKit or network test. - ADR-0023 explicitly says the network comparison predates section 5, the refusal-wait policy. Rerun the old-limits arm with this final policy before treating the network results as validation of that fix. The reported Chromium/WebKit source-quota results are author evidence; Firefox refusal handling remains unverified in the ADR.
- Security Audit reports five
quinn-protoadvisories at 0.11.15. The logged solutions require at least 0.11.18 to cover all five.Cargo.lockis unchanged against the reviewed base: these affected dependency versions were not introduced by this diff. The audit remains a separate unresolved CI gate.
Panel participation and dissent
Qwen 3.8 completed correctness and adversarial roles. DS4 completed client correctness only; its adversarial call timed out. GLM 5.3 did not return a complete verdict in either role. Thus this is 2/3 models with substantive client input, but an incomplete role panel, not consensus.
Qwen's correctness role requested changes. Two allegations do not match the pinned code: DialFailures uses EndpointFailureCache<String> (wasm_transport.rs:191), so hex parsing is not a PeerId/type mismatch; earliest-expiry cache eviction is deliberate and has an explicit passing regression test (client_engine.rs:261-330), not an inverted implementation of oldest-entry eviction. The browser-state concern remains a runtime uncertainty, not a reproduced defect. DS4 returned UNCERTAIN. Neither reviewer's speculation is being presented as a confirmed finding.
Next checks: close or explicitly disposition the audit through the normal process; complete the missing named-model roles; run section 5 on a network before ADR acceptance. No merge, deployment or ADR acceptance was performed.
Three client changes so that many browsers reading the same chunks don't pile onto one node, so a full node isn't mistaken for a dead one, and so a full holder isn't reported as a missing record. See ADR-0023.
1. Spread first reads across the close group (
client_engine/read.rs, native and browser)CLOSE_GROUP_SIZEcandidates), then fall back to nearest-first. Early reads pick among the unattempted members, and the ordinary path shuffles the close group at the front of its list.retrieve_progressivetakes the choice as a parameter. Production passesuniform_choice, and tests passnearestto keep their pinned order.2. Back off briefly from a node that declined (
browser/wasm_transport.rs,client_engine.rs)DialOutcome::Refusedwhen the node answered it:iceConnectionStatestill reads "checking".3. Wait out a refusal before reporting a record absent (
client_engine/read.rs,data/client/chunk.rs,data/network.rs)Ok(None), and the browser said "record … not found". On the testnet every failed download ended this way.network error: the nodes that may hold chunk … are busy; try again shortlyinstead of "not found".Ok(None).Compatibility. Against nodes without saorsa-transport#183, a full node stays silent, so the client times out and applies the 30-minute cooldown exactly as before. Read spreading works with any node.
Heads-up: the ADR-0020 stack (#221) also changes
client_engine/read.rs. Whichever lands second should keep the spreading rule for its holder choice.Linear issue
Closes V2-1508
Risk tier
Compatibility
BrowserNetworktrait gainsrefused_peers(), with a default that reports none, so existing adapters compile unchanged. No JS binding changes. A browser read whose holders keep refusing now rejects with a "busy" error rather than "not found".Semver impact
Test evidence
cargo fmt --all -- --checkandcargo clippy --all-targets --all-features -- -D warnings: clean.cargo clippy -p ant-core --target wasm32-unknown-unknown --no-default-features --features browser-wasm(and+test-utils)-- -D warnings: clean.cargo test -p ant-core --lib: 819 passed. New tests:busy()after 3 waits (45 s), neverOk(None).WASM suite (
node --import ./wasm-tests/setup-wasm.mjs --test ./wasm-tests/*.test.mjs): 211 passed, including the newcapacity-refusal.test.mjsandrefused-holder.test.mjs.refused-holder.test.mjs: a seed points at the only holder, which declines its first dial. The download completes after 15.1 s. Without section 3 it fails withrecord … not found, the testnet error.Real browsers, using a local 7-node devnet built against saorsa-transport#183 with limits of 2 sessions and 1 per IP:
Playwright Firefox does not establish WebRTC Direct sessions with local nodes at all (independent of this change). Its failed dials took 5.0 s, outside the 3 s window, and correctly kept the 30-minute cooldown.
Dev testnet A/B, 2026-10-13: 60 nodes and 64 browsers, each downloading the same 50 MiB file within 10 s. Full results are in the ADR's Validation section.
New dependency
none.
randwas already a dependency of ant-core.ADR
ant-node ADR-0023, in WithAutonomi/ant-node#250.
Mitigation / rollback
Revert this PR's commits. That restores nearest-first reads, the single 30-minute dial-failure cooldown, and the single 1 s retry before a read reports not found. No stored state or wire format is involved.
🤖 Generated with Claude Code