Repository navigation
fix(runs): resync watch streams when notifications are lost - #8240
Open
ChickenBenny wants to merge 3 commits into
Open
ChickenBenny wants to merge 3 commits into
ChickenBenny wants to merge 3 commits into
Conversation
Signed-off-by: ChickenBenny <zxc123benny14159@gmail.com>
Signed-off-by: ChickenBenny <zxc123benny14159@gmail.com>
Signed-off-by: ChickenBenny <zxc123benny14159@gmail.com>
ChickenBenny
force-pushed
the
fix/8193-resync-subscribers
branch
from
October 24, 2026 09:01
3646903 to
3ccb366
Compare
This branch has not been deployed
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.
Tracking issue
Closes #8193 (follow-up to #8200)
Why are the changes needed?
lib/pq sends a nil on
Listener.Notifyafter it reconnects, because NOTIFYs sent while it was down are lost (docs).processNotificationsskipped that nil, so a watcher whose run or action changed during a Postgres outage never heard about it. The console andrun.wait()stayed on the old phase with no error.Postgres doesn't replay NOTIFYs, but nothing needs them: the DB already has the new state, and each watcher knows what it is watching.
What changes were proposed in this pull request?
Three commits, easier to review separately:
Refactor, no behavior change. Subscribers become a
subscriptionstruct instead of a bare channel, and the duplicated register/unregister code of the fourWatch*functions moves intosubscribe/unsubscribe.Single-key watchers. Each subscription gets a one-slot
resyncchannel. On nil,processNotificationssignals every subscriber without blocking; repeated signals coalesce.WatchActionUpdatesandWatchRunUpdatesre-read their action or run from the DB. This coversrun.wait()/WatchActionDetails,WatchClusterEventsandWatchRunDetails.List watchers.
WatchAllRunUpdatesandWatchAllActionUpdatespass a resync on as a nil update (documented on the interface), and the service re-lists:WatchRuns(console run list) re-runs its initial list query with the same filter.WatchActions(console action tree) re-runslistAndSendAllActions, skipping actions whoseupdated_atthe stream has already seen. Without that, upserting an existing action marks it and its ancestors as changed, and the whole tree would be sent again.A failed re-list is logged and the stream stays open, the same as other read errors in these streams.
Notes for review:
WatchActionssends only what changed.WatchRunslimit: the re-list uses the stream's initial query, including its limit of 100, so it is as complete as the snapshot a fresh stream gets.Check all the applicable boxes
Related PRs
cc @AdilFayyaz