-
Notifications
You must be signed in to change notification settings - Fork 83
fix(amazonq): add jitter for websocket client re-connections #1752
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
Conversation
| this.reconnectAttempts++ | ||
| const delay = Math.min(1000 * Math.pow(2, this.reconnectAttempts), 30000) | ||
| const baseDelay = Math.min(1000 * Math.pow(2, this.reconnectAttempts), 30000) | ||
| const jitter = Math.random() * 5000 // jitter of 0 ~ 5000 milliseconds |
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.
just curious where does this 5000 come from?
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.
Just an initially chosen value to distribute the potential surge traffic across 5 seconds.
We can tune this value in the future if needed.
ramana-keerthi
left a comment
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.
It's probably less of a concern now, but we might want to add a similar jitter when we open the connection if we start to see high spikes during the start of the business day
That wouldn't help the traffic contributing from different users, since the jitter method would only help distribute the traffic across the jitter interval (i.e. it only helps actions that happen simultaneously at second-level but not for the traffic that might be relatively and consistently high for minutes or hours). But I agree something like that would help the surge traffic from the same user / machine, when opening the IDE that automatically brings up multiple previous windows. It would be something affecting all AmazonQ functionalities, and there is already a recent PR to introduce a Jitter when selecting profile (and our WorkspaceContextServer will wait until profile is selected): |
Co-authored-by: Jiatong Li <[email protected]>
Problem
We need to prevent the thundering herd problem when there is a network issue or backend issue causing a large amount of WebSocket clients disconnected.
Solution
Introduce randomness into the timing of retries to avoid simultaneous requests to re-establish WebSocket connection.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.