perf(read): share in-flight chunk fetches across concurrent reads

[Nic-dorman]

When two reads need the same chunk at the same time, both send GETs. The chunk cache is filled only once a fetch completes, so it can't catch a second request that starts while the first is in flight.

antd serving S3 ranged GETs hits this on every large download. aws-cli fetches a 64 MiB object as 8 parallel 8 MiB parts, and two things cause duplicate fetches:

• Neighbouring parts both need the chunk their shared boundary falls in.
• Every part resolves the same DataMap chunks.

This PR makes concurrent reads of a chunk share one fetch per Client:

• Sharing. chunk_get, chunk_get_from_closest_peers and the observed download path share a fetch per address and peer count (client_engine/single_flight.rs, runtime-neutral). Later callers wait for the first caller's result. Nothing is kept after the fetch ends, so caching stays the chunk cache's job.
• Failures. A failed shared fetch fails every waiter with the same error, copied by a crate-private Error::duplicate(). Each caller's own retry policy still applies afterwards: retry_data_fetch and the deferred rounds are unchanged, and a retry starts a new fetch.
• Adaptive limiter. A shared fetch feeds the adaptive fetch limiter once, if any caller sharing it observes. Waiters never record a second observation.
• Cancellation. If the caller running the fetch is dropped, it removes its entry before waking the waiters, and one of them runs the fetch in its place. If every caller is dropped, nothing is left behind.
• Diagnostics. Downloads with diagnostics enabled neither share nor lend a fetch, so their records describe only their own GETs.
• Browser read-ahead. The readers of one client read ahead through a shared pool client, so their concurrent fetches of a record now share one GET. The pool shares its fetches per record itself, so only the reader that sent the GET reports it to the reads' adaptive limiter, timed from its own start. Before this, each reader that joined would have reported the same GET again.

Linear issue

Closes V2-1433

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Proposed T1 for reviewer confirmation. The change is client-only: no node behaviour, wire protocol, stored-data format, payment or upgrade change. The only network-visible effect is that a client no longer sends duplicate GETs for reads that overlap in time. Every GET it still sends is one it would have sent before. The read path is shared by native and WASM clients, which is the case for raising it to T2.

Compatibility

• Wire: none. Same messages; concurrent reads of one chunk from one client send their GETs once rather than once per reader.
• Storage: none.
• API: no signature changes. Client::chunk_get and Client::chunk_get_from_closest_peers now share concurrent fetches of one address (documented); Error::duplicate() is crate-private.

Semver impact

• breaking
• feature
• fix

Test evidence

Unit tests (client_engine::single_flight, 7 tests):

• callers of one key share one operation;
• different keys don't share;
• a failure fails every waiter;
• a finished operation isn't reused;
• a waiter takes over when the leader is dropped;
• dropping every caller leaves nothing in flight;
• the observation flag, for each leader/waiter combination.

E2E (tests/e2e_chunk.rs::test_concurrent_chunk_gets_share_one_fetch): 8 concurrent chunk_gets with the cache empty, on the local MiniTestnet. The harness now counts each node's chunk GETs by address. Before reading the counts, the test waits until every node has counted a sentinel GET sent after the readers finish (MiniTestnet::flush_chunk_gets), so a GET still in transit is counted too.

• With this change, every node is asked for the chunk at most once.
• With sharing bypassed, one node received all 8 GETs ([0, …, 8, …, 0]) and the test fails.

Browser WASM (wasm-tests/download.test.mjs): the readers of one client share its read-ahead budget used to show that several readers read ahead by counting GETs (fetched > 11). Readers now share those GETs, so the test checks two things instead:

• the records the readers hold or fetch together exceed one reader's worth, while staying within the budget of 24;
• the 11 records they share are fetched at most once each (fetched <= 11).

Main sent more than 11 GETs here, so the new check fails if sharing is removed. All 6 read-ahead tests pass locally.

Library and lint: cargo test -p ant-core --lib passes (753). cargo fmt --check and cargo clippy -D warnings are clean for native (--all-targets --all-features) and wasm32 (browser-wasm, and browser-wasm,test-utils).

Devnet measurement. 25-node ant-devnet with Anvil; antd's byte-range branch behind an S3 gateway; cold aws s3 cp of a random 64 MiB object (20 chunks: 16 data + 4 DataMap). Three rounds alternated between two antd builds from the same source:

• base: ant-core 0.10.0 as antd pins it.
• single-flight: 0.10.0 plus this change.

antd was restarted with empty cache and limiter state before each cold run. Counts come from the node handler's debug log ("GETs received", "chunks served") and ant-core's per-GET debug log ("antd network fetches").

cold run	GETs received by nodes	chunks served by nodes	antd network fetches (beyond first)	time
base 1 / 2 / 3	88 / 82 / 99	41 / 41 / 37	41 / 35 / 36 (21 / 15 / 16)	3.00 / 2.76 / 3.01 s
single-flight 1 / 2 / 3	46 / 47 / 49	20 / 21 / 23	20 / 20 / 20 (0 / 0 / 0)	2.88 / 2.63 / 2.60 s

Where the base duplicates came from:

• 5–7 of the 16 data chunks were fetched twice: the part boundaries.
• The DataMap chunks were fetched up to 8 times: once per part.

Warm runs sent no GETs with either build, and every download was byte-identical to the upload. With sharing, the remaining GETs above one per chunk are NotFound replies to the early speculative GETs a single fetch sends; three data chunks drew 7 each in every round. That cost belongs to the single-fetch policy and is unchanged here.

Real Chromium and local nodes: passes. Its first run failed while building the pinned ant-node, a known breakage that #217 fixed; it passes now that #217 is on main.

CI: the E2E, Merkle E2E, unit and build jobs pass on Linux and macOS. They cover the full local e2e_chunk / e2e_data / e2e_file run, which host memory pressure stopped (not a test failure).

Prod compat smoke: passed on mainnet on 8 Oct, at eec1172. Public-file downloads were byte-identical to the base build and to the published hashes. Concurrent ranged reads on one client fetched every chunk once, against 35 extra fetches per read on the base build. Details: #218 (comment). The later commit 6b9df4b changes only the E2E test harness.

New dependency

none

ADR

n/a

Mitigation / rollback

Revert this commit: each caller goes back to sending its own GETs. The change is confined to Client's chunk-GET entry points and the new client_engine/single_flight.rs.

🤖 Generated with Claude Code

[dirvine]

Reviewed eec1172261c683c7e7cddec28bab109381b2d495.

Merge gate: changes requested

No blocking code defect found in the reviewed change. The remaining blocker is release evidence, not a demonstrated implementation fault:

• The PR selects T1. This repository's PR template requires CI + production compatibility smoke for T1. The PR explicitly says the smoke has not run. Attach its result before merge.
• A human reviewer must confirm the risk tier. If the reviewer selects T2, supply that tier's dev-testnet and ADR evidence instead. This review does not assign or waive the tier.

Code review

The single-flight key preserves the address and peer-count policy. Completion and cancellation remove the flight; cancellation wakes waiters so one can take over. Normal failures reach every waiter, and later calls can start a new operation. Diagnosed reads bypass sharing. Native and browser paths report the completed shared operation to the adaptive limiter only once, with the replacement leader's own start time.

Executed verification

At the exact reviewed head, on this macOS host:

• Locked ant-core library suite: 760 passed.
• Locked e2e_chunk suite, using local loopback nodes and Anvil: 10 passed, including the new sharing test.
• Three supplementary source-import concurrency probes passed, then passed in 30 repeated runs: 64 threaded callers; repeated leader cancellation; dropping a waiter without cancelling other callers. These use a disposable harness with separately resolved dependencies; they supplement the locked repository tests, not replace them.
• Native Clippy (--all-targets --all-features -- -D warnings), formatting, and diff whitespace checks passed.

Direct inspection of CI logs also confirmed the new native sharing test, 199 browser WASM tests, and 2 real-Chromium/local-node tests passed. Browser tests were not rerun locally. The author's 25-node performance measurements were not independently reproduced, and no production smoke was run by this review.

Non-blocking follow-up

The native sharing E2E test waits one second before checking the GET counters. A deterministic transport-quiescence or explicit barrier check would catch delayed duplicate traffic more reliably. No failure was observed in the executed runs.

Error::duplicate() preserves Io kind/message, but not raw OS error or its inner source. This is documented in the helper; no reachable GET-path regression was established.

Independent review and adjudication

Six substantive reviews completed: four separate GLM-5.2 API reviews (native lifecycle, browser integration, adversarial whole-diff review, and security), plus the test and release review artifacts from the worker batch. All six found no blocking code defect. The test report was recovered from disk after its worker timed out; its substantive assertions were checked against the diff and executed tests. Earlier timeouts without reports were not counted. These are separate review scopes, not six distinct model families.

The release review also identified the missing production smoke and human tier confirmation. I agree with that gate and therefore request changes rather than grant merge approval. There is no code-defect dissent. Model claims about absolute resource bounds, test determinism, and a contradictory suggestion that the smoke could be post-merge were not accepted without evidence; the repository template governs the merge requirement.

No merge, deployment, or branch modification was performed.

[Nic-dorman]

Prod compat smoke: passed

Run on 8 Oct against mainnet, from a Linux host on a home connection. Every download was checked byte for byte, and no run failed or logged an error. Both builds came from one toolchain (Rust 1.95.0):

• PR: this PR's head, eec1172.
• Base: its merge base, 9858af5.

Each run had a fresh HOME, so there was no peer cache and no adaptive-limiter warm start. Everything was download-only, with no wallet.

1. ant file download of public files

File	Size	PR	Base	SHA-256
Welcome.md	574 B	15.3 s	16.6 s	matches the published hash; identical in both builds
Mandelbrot zoom.mp4	11.6 MB	14.1 s	13.8 s	identical in both builds
LEGO video (nested DataMap)	54.9 MB	43.6 s	43.7 s	identical in both builds
64 MB of random bytes	67.1 MB	42.6 s	44.8 s	matches the published hash; identical in both builds

2. Concurrent reads of one file on one Client

A whole-file download never has two reads of one chunk in flight, so part 1 doesn't exercise the sharing. This part does. A scratch harness (source below) builds the client exactly as connect_multiaddrs does with ClientConfig::default() and bundled seeds. It then reads one public file as 8 concurrent byte ranges, the way an S3 multipart GET through antd does: each part runs data_map_fetch and then data_download_range. That covers both sharing entry points, chunk_get and the observed path.

Fetch counts come from ant-core's per-GET debug line Retrieved chunk … from peer. That line is the same in both builds. The runs alternated between base and PR builds.

File	Build	Chunk retrievals (unique chunks)	Duplicates	Read time	Bytes
64 MB random	base, 2 runs	56 / 56 (21)	35 / 35	32.0 / 35.0 s	✅ hash matches
64 MB random	PR, 2 runs	21 / 21 (21)	0 / 0	29.8 / 28.0 s	✅ hash matches
LEGO video	base, 2 runs	53 / 53 (18)	35 / 35	28.3 / 28.3 s	✅ same hash as part 1
LEGO video	PR, 2 runs	18 / 18 (18)	0 / 0	27.2 / 26.7 s	✅ same hash as part 1

The base build's 35 duplicates have the same shape on mainnet as in the devnet measurement in the description:

• The four DataMap-level chunks (323–430 B) were fetched 8 times each, once per part.
• Seven ~4 MiB data chunks were fetched twice each, at the part boundaries.

With the PR, each chunk is fetched once. That removes about 29 MB of extra transfer per read, roughly 44% of the 64 MB file. Read times were no slower; the small difference is within run-to-run noise.

Logs: no ERROR in any run. Both builds logged the same WARN sources at similar counts: DHT dial failures ([STEP 1b] … dial failed) and the local interface scan timeout. Bootstrap-seed connect failures were excluded from both. The PR introduced no new warning types.

This smoke ran at eec1172. The commit after it, 6b9df4b, changes only the E2E test harness (below).

Review follow-up: the sharing test no longer sleeps

6b9df4b replaces the fixed one-second wait in test_concurrent_chunk_gets_share_one_fetch with an explicit barrier. After the readers finish, MiniTestnet::flush_chunk_gets sends every other node a sentinel GET from the client's node. It then waits until each node has counted it, and fails after 10 s.

A GET's send returns only once the node's QUIC stack has acknowledged the stream. So every GET sent before the barrier has reached its node before the sentinel goes out, and nodes accept one peer's streams in the order they were opened. One caveat: a node reads each stream in its own task. Counting therefore follows arrival order in practice, not as a strict guarantee.

Checked on a Linux host:

• Clippy: clean, native, --all-targets --all-features -- -D warnings.
• e2e_chunk: the full suite passes.
• Sharing test: passed 10 of 10 runs.
• With sharing bypassed: the test still fails, and one node receives all 8 GETs ([0, 0, 0, 0, 8, 0, …]).

The risk tier is still for a human reviewer to confirm.

Harness (ant-core/examples/smoke218.rs, scratch, not committed)

//! PR #218 prod compat smoke harness (scratch, not for merge).
//!
//! Reads one public file from mainnet as `parts` concurrent byte ranges on a
//! single `Client`, the way an S3 multipart GET through antd does: every part
//! fetches the DataMap itself, then reads its range. Writes the reassembled
//! bytes to `out` so the caller can checksum them.
//!
//! usage: smoke218 <public address hex> <size> <parts> <out file>

use ant_core::data::{Client, ClientConfig};
use std::io::Write;
use std::time::Instant;

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    tracing_subscriber::fmt()
        .with_env_filter(tracing_subscriber::EnvFilter::from_default_env())
        .with_ansi(false)
        .with_writer(std::io::stderr)
        .init();

    let args: Vec<String> = std::env::args().collect();
    anyhow::ensure!(
        args.len() == 5,
        "usage: smoke218 <address> <size> <parts> <out>"
    );
    let address: [u8; 32] = hex::decode(&args[1])?
        .try_into()
        .map_err(|_| anyhow::anyhow!("address must be 32 bytes"))?;
    let size: usize = args[2].parse()?;
    let parts: usize = args[3].parse()?;
    let out = &args[4];

    let t0 = Instant::now();
    let bootstrap = ant_core::config::resolve_bootstrap_multiaddrs(&[], None)?;
    let client = Client::connect_multiaddrs(&bootstrap, ClientConfig::default()).await?;
    let connect = t0.elapsed();

    let part = size.div_ceil(parts);
    let t1 = Instant::now();
    let reads = (0..parts).map(|i| {
        let client = &client;
        async move {
            let started = Instant::now();
            let data_map = client.data_map_fetch(&address).await?;
            let bytes = client.data_download_range(&data_map, i * part, part).await?;
            anyhow::Ok((bytes, started.elapsed()))
        }
    });
    let results = futures::future::try_join_all(reads).await?;
    let read = t1.elapsed();

    let mut file = std::fs::File::create(out)?;
    let mut total = 0;
    for (bytes, _) in &results {
        file.write_all(bytes)?;
        total += bytes.len();
    }
    let per_part: Vec<String> = results
        .iter()
        .map(|(_, took)| format!("{:.2}", took.as_secs_f64()))
        .collect();
    println!(
        "RESULT connect_s={:.2} read_s={:.2} bytes={total} parts={parts} part_s=[{}]",
        connect.as_secs_f64(),
        read.as_secs_f64(),
        per_part.join(",")
    );
    anyhow::ensure!(total == size, "read {total} bytes, expected {size}");
    Ok(())
}

Run as RUST_LOG="warn,ant_core::data::client::chunk=debug" smoke218 <address> <size> 8 <out>. Addresses: 64 MB random f6eeba94008f493a8518186b0647df6d39dbde75f41d5b79549e9a4e63a1ab43 (size 67108864), LEGO 41852ea4bc68193565974e39552ce3ac1b32edddc7e9695910eaff1fbc34645c (size 54903201).

[dirvine]

Focused follow-up review of 6b9df4b1862d67b19c8127e792ebc3b6560b31ab, against the previously reviewed eec1172261c683c7e7cddec28bab109381b2d495. This is a delta review, not a fresh full panel of the original production change.

The original release-evidence blocker is resolved:

• The author's production compatibility smoke report covers mainnet public-file downloads and concurrent ranged reads on one client. It reports matching base/PR bytes, published-hash matches for the specified files, and no new warning types. I inspected the report; I did not rerun mainnet tests.
• The smoke ran at eec1172. The complete subsequent diff changes only ant-core/tests/e2e_chunk.rs and ant-core/tests/support/mod.rs; production source, manifests and lockfiles are unchanged. The smoke therefore remains applicable to the production change.
• Chris explicitly confirmed T1 in the originating review-request conversation.

The test-only follow-up replaces the fixed sleep with sentinel GETs and waits for each node to count them. This is a practical improvement, not a strict cross-stream processing barrier; the author discloses that limitation. It remains a non-blocking test limitation, not a demonstrated production defect.

All 21 current check entries pass. I inspected CI logs for the sharing test on Linux and macOS, 205 passing generated-WASM tests, and 2 passing real-Chromium/local-node tests. Those CI jobs tested synthetic merge 353537b containing this head, not a raw-head checkout. No new local Rust tests or production smoke were executed in this follow-up.

Disposition: no remaining blocker found in the follow-up code or supplied T1 evidence, but formal approval is held. Qwen 3.8 and DS4 completed independent focused reviews and both returned APPROVE. GLM 5.3/OpenRouter returned incomplete output on both bounded attempts. Participation is therefore 2/3 reviewed — provisional, not full consensus. The standing review gate does not permit approval while that required seat is incomplete. This is a review-tooling limitation, not a request for another code change.

The earlier missing-smoke and human-tier findings should not be treated as outstanding. No merge, deployment or branch modification was performed.