Exponential backoff on reconnection attempts #403
Conversation
|
Exponential backoff if definitely a good idea. I'm all for this. My initial thoughts are that that there needs to be some kind of internal log within the Sarus class that tracks all the actions taken by the instance (attempt to connect, connect success, connection closes, etc). Then, the log has the historical context of what has been happening and when. We then have an option to enable exponential backoff in Sarus, and when this is enabled, it will use the log to analyse the connection history and determine how much to wait until the next connection retry. In theory it just needs to modify the value of I also realise that we might want to have fine-grained control over the exponential back-off strategy (e.g. how much to increase the wait by, whether to set an upper limit that the exponential back-off wait can reach). Good feature to add 👍 |
|
Thanks for the feedback 😄 Yes, keeping track of the internal state is my line of thinking as well. #404 is supposed to achieve this by modelling Sarus as a FSM. Regarding the configurability: Backoff rate and upper limit are configurable and required at the moment. It could be useful to make some of these parameters not required in the future, but in order to get the feature out there, working and tested it's useful to have a first working version out soon. I intend to use this feature as soon as possible in Projectify (https://github.com/jwp-consulting/projectify-frontend) |
|
@justuswilhelm hi, sorry for taking so long to get back on this. I'm happy for this PR to get merged. And I will be taking a look at the other PR now. |
justuswilhelm
force-pushed
the
exp-backoff
branch
6 times, most recently
from
037e01b to
568b9de
Compare
|
Hi @paulbjensen, thanks for merging my other PRs :) Can you please review this PR? Divided over a few commits I did the following:
The state tracking is a bit complicated since there was already a state machine lurking in Sarus. The functions connect(), reconnect(), disconnect(), onclose(), onopen() already serve as springboards for state transitions. I hope the test coverage is sufficient, please let me know if you have any feedback. |
By turning state from simple string literals to full objects, we can add annotations that are specific to one state. Use case: When retrying connections, we could just add a connectionAttempt?: number; property, but then we would have to check if it is undefined or not, and if we are not reconnecting currently and connectionAttempt? is set, does that mean the Sarus client entered an invalid state? By guaranteeing certain properties to only ever be present when the state property has the right kind, we can avoid a whole class of bugs just by relying on the type system.
Previously, it said "Function", but this was probably meant in reference to the callbacks called further below. Refering to the MDN docs on WebSocket Interfaces [^1], one can see that there are three possible events: - Event, - CloseEvent, and - MessageEvent [^1][https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API#interfaces]
Allow configuring and enabling exponential backoff for failing connections, instead of relying on reconnections in regular intervals. Exponential backoff can be configured in order to lighten server load if multiple clients are reconnecting. The new configuration parameter is called `exponentialBackoff` and requires two parameters `backoffRate` and `backoffLimit`.
This is used to calculate the exponentially increasing delay. The function is used to calculate the reconnection delay - but does not keep track of the current reconnection attempt. Keeping track of the current reconnection attempt is reserved for the next few commits.
Track how many times Sarus has already tried to (re)connect. When exponential backoff is enabled, use the stored number of connection attempts to calculate the exponential delay.
It turns out we don't need it here after all
justuswilhelm
force-pushed
the
exp-backoff
branch
from
074c714 to
6e88d53
Compare
justuswilhelm
changed the title
|
I've rebased the master branch on this branch, as there was a merge conflict in src/index.ts |
|
Hi, sorry if I catch you during a busy time. Please let me know if anything else is needed to make this PR mergable. Thanks again for all the input so far. |
|
Sorry, I'll review this PR this weekend. |
DEPENDS ON #402
DEPENDS ON implementation of a state machine, or other means to track if connection is failing.
The problem: reconnecting with a default delay of 1s can result in a lot of additional traffic on a server.
Scenario:
The solution is to implement exponential backoff. Please refer to the readme for an exact description.
Question to @paulbjensen: If a connection is failing, I need to track the amount of failed connection attempts. To do that, I first need to know if a connection is failing. A state machine is usually a good idea to track the current state of a connection. I will prepare another to PR to lay the groundwork for this PR here. Do you have any other idea on how to implement this/is there anything else I can use in the code?