-
Notifications
You must be signed in to change notification settings - Fork 19
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
add exponential retry mechanism for rpc requests using utility function #593
Open
najeal
wants to merge
8
commits into
ava-labs:main
Choose a base branch
from
najeal:retry-exponential
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
eb7887f
add exponential retry mechanism for rpc requests using utility function
najeal 430fcaf
add tests to retry mechanism
najeal 43fc8b2
update backoff mechanism + use it in additional places
najeal b266ed6
backoff fix test
najeal 58d18ec
Merge remote-tracking branch 'upstream/main' into retry-exponential
najeal 89dc5b5
fix: nits
najeal e91355d
rename maxElapsedTime consts has timeout
najeal b5dcb34
Merge branch 'main' into retry-exponential
geoff-vball File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
rename maxElapsedTime consts has timeout
- Loading branch information
commit e91355d4500824f4e88c4cbc91ff5ee3618157d3
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ type blsSignatureBuf [bls.SignatureLen]byte | |
const ( | ||
// Maximum amount of time to spend waiting (in addition to network round trip time per attempt) | ||
// during relayer signature query routine | ||
signatureRequestMaxElapsedTime = 20 * time.Second | ||
signatureRequestTimeout = 20 * time.Second | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, we were waiting 20s between checks, which we did 10 times, so lets make this ~200s There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the same calcul here |
||
) | ||
|
||
var ( | ||
|
@@ -327,7 +327,7 @@ func (s *SignatureAggregator) CreateSignedMessage( | |
return errNotEnoughSignatures | ||
} | ||
|
||
err = utils.WithRetriesTimeout(s.logger, operation, signatureRequestMaxElapsedTime) | ||
err = utils.WithRetriesTimeout(s.logger, operation, signatureRequestTimeout) | ||
if err != nil { | ||
s.logger.Warn( | ||
"Failed to collect a threshold of signatures", | ||
|
This file contains 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
This file contains 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
This file contains 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
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.
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.
We were making a max of 5 attempts with 10s in between each. Can we make the timeout 60s to match closer?
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.
I think we were waiting
time.Duration(signatureRequestRetryWaitPeriodMs/maxRelayerQueryAttempts) * time.Millisecond
->
(10_000 / 5) milliseconds
->2_000
milliseconds each round