Skip to content
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

Retry fetch logs #317

Merged
merged 4 commits into from
Jun 5, 2024
Merged

Retry fetch logs #317

merged 4 commits into from
Jun 5, 2024

Conversation

cam-schultz
Copy link
Collaborator

Why this should be merged

The websocket connection serving new blocks to process and the RPC request to fetch logs from a block containing Warp messages may be served by different nodes that have indexed different heights. If the node serving the RPC request is behind the node serving blocks, then the FilterLogs call will fail. Adding retries the the FilterLogs RPC request works around this.

How this works

See above

How this was tested

CI

How is this documented

N/A

types/types.go Outdated Show resolved Hide resolved
types/types.go Outdated Show resolved Hide resolved
michaelkaplan13
michaelkaplan13 previously approved these changes Jun 4, 2024
geoff-vball
geoff-vball previously approved these changes Jun 5, 2024
@cam-schultz
Copy link
Collaborator Author

I also added a goreleaser change to not mark rc's as latest in dockerhub.

Copy link
Contributor

@bernard-avalabs bernard-avalabs left a comment

Choose a reason for hiding this comment

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

LGTM. Just a suggestion about using an exponential back off if the height difference can be significant.

return logs, nil
}
if i != numRetries-1 {
time.Sleep(retryInterval)
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to consider some type of exponential back off between retries if the height difference between nodes can potentially be significant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As part of the nonce ordering fix, I'm going to implement a generic retry helper, and will consider adding exponential backoffs there.

@cam-schultz cam-schultz merged commit 528eca4 into main Jun 5, 2024
7 checks passed
@cam-schultz cam-schultz deleted the retry-fetch-logs branch June 5, 2024 19:32
@michaelkaplan13 michaelkaplan13 requested a review from feuGeneA June 10, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants