-
Notifications
You must be signed in to change notification settings - Fork 334
cannon: Replace message count mechanism with detecting the end of initial sync #4631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
…tial sync The message count recieved in the queue info gotten as response of declaring the queue may include some expired messages which have not yet made it to the head of the queue. This makes the message count mechanism useless to the clients for detecting the end of initial sync. This commit detects end of initial sync based on sending a transient notification in the queue. If we recieve this notification back, it would mean that the queue is empty and the initial sync is complete. The timing of sending this notification must be carefully chosen, it is sent in one of these two conditions: 1. If the message count in the queue info is 0, the queue must be empty as the expired messages are already considered expired when they reach the head of the queue. So its a good time to publish our end-of-initial-sync notification. If the queue is indeed empty we'll recieve it and forward it to the client, letting it know that it can consider itself up to date. 2. If number of unacked messages becomes 0 right after recieving an acknowledgement and the initial sync hasn't already completed, this could mean either the queue is empty or there is some delay in receiving the next message from RabbitMQ. Here we publish our end-of-initial-sync notification. If there was a delay in receiving the next message from RabbitMQ, the consumer thread will increase the count for unacked messages and we'll get another opportunity to check this. Otherwise, the consumer thread will recieve this end-of-initial-sync notification and forward it to the client.
The IORef is being manipulated from two different threads, so it should use the atomic version of the function.
Description in the comment
…axChannels These option were implemented previously but we forgot to add them in the helm chart.
Use configurable TTL with default value set to 28 days like for normal notifications.
battermann
approved these changes
Jul 1, 2025
services/cannon/src/Cannon/WS.hs
Outdated
| reqId = RequestId defRequestId | ||
| in Env {..} | ||
|
|
||
| -- Env leh lp (RequestId defRequestId) |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comment
Instead of expecting it to be passed in
4 tasks
This was referenced Jul 8, 2025
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.
https://wearezeta.atlassian.net/browse/WPB-18436
The message count recieved in the queue info gotten as response of declaring the
queue may include some expired messages which have not yet made it to the head
of the queue. This makes the message count mechanism useless to the clients for
detecting the end of initial sync.
This PR allows clients to detect the end of initial sync by inserting a synchronization marker upon connecting to the websocket using a query param. Once they receive this synchronization marker, they will know that all the notifications which existed in the queue before they connected to the websocket have been consumed.
Checklist
changelog.d