Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: implement backoff retry policy for websocket handler #3600

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

satyajeetkolhapure
Copy link
Contributor

@satyajeetkolhapure satyajeetkolhapure commented Sep 1, 2022

Web socket handler receives closed connection event more frequently than expected from infura server. So to make ganache more robust a backoff retry mechanism is implemented.

Retry configuration - 3 retries with 2 seconds initial retry and with exponential attempts followed.

When connection is closed, in flight requests are stored temporarily and processed once connection retry attempt succeeds.

Note: Once connection is successful, the retry counter is reset to initial counter.

@satyajeetkolhapure satyajeetkolhapure self-assigned this Sep 1, 2022
@satyajeetkolhapure satyajeetkolhapure changed the title fix: implement backoff retry policy for web socket handler Draft : implement backoff retry policy for web socket handler Sep 1, 2022
@satyajeetkolhapure satyajeetkolhapure changed the title Draft : implement backoff retry policy for web socket handler draft : implement backoff retry policy for web socket handler Sep 1, 2022
});
this.connection.binaryType = "nodebuffer";
this.connection.onclose = onCloseEvent;
this.open = this.connect(this.connection, logging);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a inFlightRequests that may need care/attention here. I am not sure if the desired behavior would be to kill/cancel them or attempt to rebroadcast if/once the socket reconnects and if that's possible. I don't know this part of code. I would assume the latter but @davidmurdoch will need to clarify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @davidmurdoch : Will this change have any impact on inFlightRequests?

Copy link
Contributor Author

@satyajeetkolhapure satyajeetkolhapure Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tenthirtyone , @davidmurdoch : In flight requests are now stored when connection is closed and processed once connection retry attempt succeeds

@davidmurdoch davidmurdoch marked this pull request as draft September 8, 2022 16:54
@davidmurdoch davidmurdoch changed the title draft : implement backoff retry policy for web socket handler fix: implement backoff retry policy for websocket handler Sep 8, 2022
@satyajeetkolhapure satyajeetkolhapure marked this pull request as ready for review October 26, 2022 14:33
Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a review, I just had a question.

@MicaiahReid
Copy link
Contributor

Would it be possible to get tests added for this change?

@satyajeetkolhapure
Copy link
Contributor Author

Would it be possible to get tests added for this change?
Yes. I am going to add few tests to check InFlightRequests too.

@satyajeetkolhapure satyajeetkolhapure force-pushed the refactor/implement-backoff-retry-policy-in-websocket branch from 26586c8 to 7cdc04e Compare December 13, 2022 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Stalled
4 participants