Let channel types opt out of the event-by-event /sync replay after a long outage - #232
Open
harlan wants to merge 3 commits into
Open
Let channel types opt out of the event-by-event /sync replay after a long outage#232harlan wants to merge 3 commits into
harlan wants to merge 3 commits into
Conversation
On reconnect the SDK catches up by calling /sync with the timestamp of the last event received before the disconnect. The server refuses the request when the gap is too large — code 4 / HTTP 400, "Too many events to sync, please use a more recent last_sync_at parameter" — which the ~1000-event limit reaches long before the 30-day bound the guard above it checks. That cap counts events, not messages, across every cid passed in the one call, so a single message can contribute a message.new plus a message.read per member. A player who leaves the app backgrounded on a busy channel and returns hours later hits it every time. The failure had no handler. FetchAndProcessEventsSinceLastReceivedEvent is called fire-and-forget through LogIfFailed, so the exception reached the logger and nothing else: the watched channels kept the state they had before the disconnect, missing every message since, until something unrelated happened to re-fetch them. And _disconnectionLastEventReceivedAt stayed stale, so the next reconnect failed exactly the same way. In one of our production titles this is 6k+ such warnings across 4.2k users in 30 days; for a live room or an open feed it is a silent correctness gap, not just noise. Now the low-level client drops the stale sync point and rethrows, and RestoreStateLostDuringDisconnect catches the input error and re-watches every watched channel — the same full state fetch the initial watch does, which is the only way to recover once the events are past replay. Each channel is attempted independently: this runs after the stale sync point has been dropped, so it is the only recovery this reconnect gets, and a single failure escaping the loop would leave every remaining channel silently stale for the rest of the session. Failures are expected here, not exotic — a channel torn down while offline returns 403 on every read, and a long watched list can trip a 429 part-way. Known limitation, flagged in a comment: GetOrCreateChannelWithIdAsync is get-OR-create, so re-watching a channel that was hard-deleted while offline recreates it server-side as an empty channel. Fixing that properly means consulting SyncResponse.InaccessibleCids — already returned by /sync and currently ignored — to skip channels the server says are gone, rather than discovering it one 403 at a time. Happy to take that on in this PR if you would rather not merge the get-or-create behavior.
The re-watch recovery replaces a channel's messages wholesale and raises no per-message events for the window it replaced, so a consumer that rebuilds its UI from message events alone keeps rendering the rows it had before the disconnect - the recovery restores local state but nothing tells the UI to read it. Raise IStreamChatClient.ChannelsRewatched with the re-watched channels so consumers can rebuild from IStreamChannel.Messages. Raised even when some channels failed to restore: the ones that succeeded did have their state replaced, and rebuilding from a failed channel's unchanged list is harmless.
After a long outage the reconnect catch-up replays every missed event through the consumer's handlers one at a time. For a channel whose consumer only ever displays a bounded latest window, that replay ends at the same visible state a single re-watch request would have produced - so on a busy channel the resume spends its cost on work that is immediately discarded. SetRewatchOnReconnectChannelTypes lets the integrator name the channel types that should be restored by a bounded re-watch instead when the outage exceeds maxSyncReplayGap (default 60s). Types that are not listed keep the precise replay, and short outages always replay, where the backlog is small and the replay keeps consumers seamless. Unset by default, so behavior is unchanged. The outage is the age of the sync point - the last handled event - and not the time since the Disconnected transition. Detection can lag the real outage by its entire length: a mobile OS suspends the process while backgrounded, so the dead socket is only noticed on resume and a transition stamp would measure seconds for an hours-long background, never engaging this path on the platform that needs it. Health check events advance the watermark every ~30s while connected. The re-watch also no longer depends on the replay's outcome: a transient /sync failure is logged and the re-watch still runs, rather than leaving the opted-in channels with no recovery for the rest of the session.
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
1 participant
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
After a long outage the reconnect catch-up (
RestoreStateLostDuringDisconnect→/sync) replays every missed event through the consumer's handlers, one at a time. For a channel whose consumer only ever displays a bounded latest window — a livestream-style room, an announcement channel — that replay ends at exactly the visible state a single re-watch request would have produced.The cost is real, because per-event handling is not free in a real consumer: in our app each event drives a persistence write, profile resolution and a feed reload. An app that spends an hour backgrounded on a busy channel resumes into hundreds of those, and the newest N are all the UI will ever show.
Change
SetRewatchOnReconnectChannelTypes(channelTypes, maxSyncReplayGap)lets the integrator name the channel types that should be restored by a bounded re-watch when the outage exceededmaxSyncReplayGap(default 60s) — the same latest-page state fetch the initial watch performs, one request per channel, reported viaChannelsRewatched(added in #227) so consumers rebuild fromIStreamChannel.Messages.Unset by default, so nothing changes unless you opt in. Types you don't list keep the precise event replay, which is the right default for anything persisting full history locally. Short outages always replay: the backlog is small and the replay keeps consumers seamless.
Two supporting details, both in the diff:
The outage is the age of the sync point, not the time since
Disconnected. Detection can lag the real outage by its entire length — a mobile OS suspends the process while the app is backgrounded, so the dead socket is only noticed on resume. A stamp taken at theDisconnectedtransition would measure seconds for an hours-long background, and this path would never engage on exactly the platform that needs it. Health check events advance_lastEventReceivedAtevery ~30s while connected, so the sync point tracks the real quiet period. Exposed as the internalTimeSinceDisconnectSyncPoint.The re-watch no longer depends on the replay's outcome. A transient
/syncfailure (5xx, or the network dropping again mid-reconnect) previously propagated to nothing but the caller's fire-and-forget logger. With this change that would also skip the re-watch, leaving the opted-in channels with no recovery for the rest of the session — so it is logged and the re-watch still runs. The replay channels keep their sync point, so the next reconnect retries them.Relationship to #225
Complementary, no overlap — different files, and neither needs the other:
Together the remaining replay (the channels that still need every event) is both smaller and paced. Either can land first.
Open questions
SetRewatchOnReconnectChannelTypesis an imperative setter, but this is policy configuration and you have twice put that kind of knob onIStreamClientConfig—OptimisticMessageInsert, andDefaultMessageCacheWindow+OverrideMessageCacheWindowin Feature/uni 192 implement cache limit option #230. Happy to reshape it the same way (MaxSyncReplayGapon the config, opt-out per channel type or per channel) if you'd prefer the consistency; I led with the setter only because the set is naturally client-wide and read once per reconnect.catch (Exception)around the replay is deliberately broad — the point is that no replay failure should skip the re-watch — but I'm happy to narrow it to the transport/HTTP exception set if you'd rather not swallow everything./synccall, so their recovery never advances the watermark; it stays at the pre-outage value until live traffic resumes. That's benign — the re-watch is idempotent and bounded, so the worst case is re-watching once more on a quiet reconnect — but it's the sharp edge in this design and worth a second opinion.StreamChatClientTeststoday; glad to add one if you want it.Testing
The equivalent change has been running in our vendored copy of the SDK against a production workload, with the game-side consumer wired to it. Compiles clean against the v5.7.0 tree.