provider: bump notify_streaming/client_streaming update cap 32 -> 8192 (or make configurable)#232
Open
hasantr wants to merge 1 commit into
Open
provider: bump notify_streaming/client_streaming update cap 32 -> 8192 (or make configurable)#232hasantr wants to merge 1 commit into
hasantr wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
src/provider/events.rs, four call sites (inEventSender::request_received_inner) hardcodeupdate_cap = 32: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()usestx.try_send(...)which then silently dropsRequestUpdate::Progressevents.Downstream consumers (in our case a chat app with sender-side file-transfer progress UI) observe progress events stuck at
Startedfollowed directly byCompleted, with no intermediateProgressevents for the entire transfer. Smaller transfers see noProgressevents at all (consumer task hasn't drainedStartedbefore 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 twoActivevariants (NotifyLog,InterceptLog) are the critical path; the twoDisabledvariants (Notify,Intercept) are bumped only for consistency since their update channel is never read.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), withEventMask { get: RequestMode::NotifyLog, .. }. CountRequestUpdate::Progressitems observed:cap=32): ~30-60 events for a 100 MB transfer (highly variable, system load dependent), or 0 events for small transfers.cap=8192): all events delivered (~1600 for 100 MB).Testing
cargo check --libclean.RequestUpdate::Progressevent stream (26000+ events), driving smooth sender-side progress UI.Notes for maintainers
I noticed the
Frando/provider-events-refactorbranch is in flight — feel free to close this if that refactor will supersede the currentEventSenderdesign. In that case, please consider exposing the per-request update channel capacity as a configurable parameter in the new design.