fix(mobile): actually throttle unread catch-up at startup - #4444
Open
CryptoJones wants to merge 1 commit into
Open
fix(mobile): actually throttle unread catch-up at startup#4444CryptoJones wants to merge 1 commit into
CryptoJones wants to merge 1 commit into
Conversation
Invoking an async function in Dart starts it immediately, so building a List<Future> up front dispatched every per-channel history REQ at once and left the batch loop only awaiting work already in flight. The batchSize of 5 never limited concurrency. A member of N channels therefore issued N concurrent history REQs during startup, on top of the per-channel live subscriptions, presence, and the section sync. On a relay with the default fixed-window WS admission budget (10 events/sec over a 5s window) the overflow was rejected with "rate-limited: quota exceeded", and the failed catch-ups then retried, re-spending the same budget. Collect thunks instead and invoke each batch as it is awaited, so at most batchSize requests are ever in flight. Signed-off-by: Aaron K. Clark <cryptojones@owasp.org>
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
_catchUpUnreadEventsbuilt aList<Future>before awaiting it:Invoking an
asyncfunction in Dart starts it right away, so every per-channel historyREQwas dispatched during the loop that builds the list. The batch loop below only awaited work that was already in flight, which madebatchSize = 5a no-op for concurrency.A member of N channels therefore issued N concurrent history requests at startup, on top of the per-channel live subscriptions, presence, and the section sync.
Impact
On a relay using the default WS admission budget (
BUZZ_RATE_LIMIT_HUMAN_WS_EVENTS_PER_SEC = 10over the fixed 5sWS_BURST_WINDOW_SECS), the overflow is rejected:The failed catch-ups then retry, re-spending the same fixed window, so a single overflow becomes a sustained failure loop. On a 14-channel account the app launches, fails to populate, and retries indefinitely — which presents to the user as the app hanging or crashing on open.
The relay-side comment on
WS_BURST_WINDOW_SECSnotes the window was sized for desktop startup, which establishes fewer simultaneous subscriptions than mobile.Fix
Collect thunks rather than futures and invoke each batch as it is awaited, so at most
batchSizerequests are ever in flight. No behavioural change beyond concurrency.Testing
dart format— cleanflutter analyze— no issuesflutter test— 905 pass. One pre-existing failure inchannel_detail_page_test.dart("keeps follow mode off while a tall newest message stays visible") which reproduces identically on a cleanorigin/maincheckout and is unrelated to this change.Notes
This reduces the startup burst but does not eliminate it — the per-channel live subscriptions still fan out from a separate path. Filed #4440 for the relay side, where the WS admission budget always uses the human per-second limit even for agents and has no agent tier.
Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/