Skip to content

provider: bump notify_streaming/client_streaming update cap 32 -> 8192 (or make configurable)#232

Open
hasantr wants to merge 1 commit into
n0-computer:mainfrom
hasantr:sezgi-update-cap
Open

provider: bump notify_streaming/client_streaming update cap 32 -> 8192 (or make configurable)#232
hasantr wants to merge 1 commit into
n0-computer:mainfrom
hasantr:sezgi-update-cap

Conversation

@hasantr
Copy link
Copy Markdown

@hasantr hasantr commented May 21, 2026

Problem

In src/provider/events.rs, four call sites (in EventSender::request_received_inner) hardcode update_cap = 32:

RequestUpdates::Active(client.unwrap().notify_streaming(Notify(msg), 32).await?)
// ...and the analogous client_streaming(msg, 32) for InterceptLog

For high-throughput transfers (>10 MB on LAN, or any transfer with fast producer / slow consumer drain mismatch), the per-request update channel fills up almost instantly. RequestTracker::transfer_progress() uses tx.try_send(...) which then silently drops RequestUpdate::Progress events.

Downstream consumers (in our case a chat app with sender-side file-transfer progress UI) observe progress events stuck at Started followed directly by Completed, with no intermediate Progress events for the entire transfer. Smaller transfers see no Progress events at all (consumer task hasn't drained Started before the inner channel closes).

Proposal

This PR bumps the hardcoded default from 32 to 8192 in all four call sites of EventSender::request_received_inner. The two Active variants (NotifyLog, InterceptLog) are the critical path; the two Disabled variants (Notify, Intercept) are bumped only for consistency since their update channel is never read.

  • 8192 × ~64 byte event ≈ 500 KB peak per-request RAM
  • 100 MB transfer @ 64 KB chunks ≈ 1600 events — comfortably fits

Happy to extend with a configurable parameter on EventSender::channel() instead of a hard-coded value if maintainers prefer.

Reproduction

Loopback file-to-file transfer using BlobsProtocol::add_path + downloader.download(hash), with EventMask { get: RequestMode::NotifyLog, .. }. Count RequestUpdate::Progress items observed:

  • Before patch (cap=32): ~30-60 events for a 100 MB transfer (highly variable, system load dependent), or 0 events for small transfers.
  • After patch (cap=8192): all events delivered (~1600 for 100 MB).

Testing

  • cargo check --lib clean.
  • Field-tested in sezgi-messaging on Android arm64 + Windows x86_64. A 428 MB video transfer now produces a continuous RequestUpdate::Progress event stream (26000+ events), driving smooth sender-side progress UI.

Notes for maintainers

I noticed the Frando/provider-events-refactor branch is in flight — feel free to close this if that refactor will supersede the current EventSender design. In that case, please consider exposing the per-request update channel capacity as a configurable parameter in the new design.

The hardcoded update channel capacity of 32 in EventSender's request
mode initializers (notify_streaming/client_streaming, 4 call sites)
causes RequestUpdate::Progress events to be dropped via try_send when
transfer throughput exceeds consumer drain speed (e.g. >10 MB local
transfers).

Bumping to 8192 (~500 KB peak per-request RAM) gives ~250x headroom
and matches typical chunked transfer event volume (1600 events for
100 MB at 64 KB chunks).

Downstream sezgi-messaging observes sender-side progress UI stuck at
0% without this patch — RequestMode::NotifyLog Active variant is the
critical path; InterceptLog Active also bumped for consistency. Notify
and Intercept (Disabled variants) bumped for tidiness only — their
update channel is never read.
@n0bot n0bot Bot added this to iroh May 21, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🚑 Needs Triage

Development

Successfully merging this pull request may close these issues.

1 participant