Chunk fetches continue after every waiter has cancelled, with an undocumented bound

[vilenarios]

Summary

ArweaveCompositeClient.getChunkByAny deliberately does not cancel the shared fetch when a caller aborts, so that other waiters on the same chunkPromiseCache entry still receive the result. The consequence is that work continues after every waiter has gone, and nothing documents or records the bound.

Raised by CodeRabbit on #878; filed here because it is independent of that PR's two-line default change.

Mechanism

• The read-through function calls peerGetChunk with no signal (composite-client.ts:359-371).
• getChunkByAny races only the caller's await against the abort signal (:1536-1546), with the comment explaining why: cancelling would rob other waiters.
• peerGetChunk then loops sequentially Math.max(peerSelectionCount, retryCount) times at PEER_CHUNK_REQUEST_TIMEOUT_MS = 500 (:105).

Production wiring passes ARWEAVE_PEER_CHUNK_GET_PEER_SELECTION_COUNT (default 10) and ARWEAVE_PEER_CHUNK_GET_MAX_PEER_ATTEMPT_COUNT (default 5) (system.ts:276-278), so the bound is 10 attempts, about 5 seconds of detached work per abandoned request. Not the 50 attempts the constructor default suggests.

Why it is worth attention

Detached work is the common case, not the exception. Over a 146-hour window on a production gateway:

• 652,202 peer-origin chunk requests reached offset resolution
• 99.30% were cancelled by the caller before completion
• 0.31% delivered any bytes

Callers give up quickly by design: PEER_REQUEST_TIMEOUT_MS in ar-io-chunk-source.ts is 1 second, while the serve deadline is 12 seconds. So a large share of chunk retrieval work outlives every waiter.

The current design is defensible, because the detached fetch populates the cache and a later request may hit it. What is missing is that the bound is implicit, nothing measures how much work is detached, and the two entry points (getChunkDataByAny, getChunkMetadataByAny) both funnel through the same path with no coverage for the cancellation case.

Options

1. Document the bound and leave the behavior alone. Smallest change: state in getChunkByAny that a cancelled caller leaves up to max(selection, attempts) x 500ms of detached work, and that this is intentional cache-warming.
2. Record it. A counter for fetches that outlived all waiters would turn "the common case" from an inference into a number, and would show whether the cache-warming argument actually pays off.
3. Bound it explicitly. Give the shared fetch its own deadline, independent of any caller, so the detached tail cannot grow if the peer-attempt defaults are raised.
4. Cancel when the last waiter leaves. Reference-count the promise cache entry. Correct in principle, most invasive, and it forfeits the cache-warming benefit.

Option 2 first would tell us whether 3 or 4 is worth the risk. Tests for the cancellation path on both entry points belong with whichever is chosen.

🤖 Generated with Claude Code