-
Notifications
You must be signed in to change notification settings - Fork 3k
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
op-node: Modify p2p unsafe sync rate limits #5684
Conversation
|
✅ Deploy Preview for opstack-docs canceled.
|
1be8b79
to
3b359ff
Compare
The changes look fine to me, but are the comments of @ajsutton resolved or not? |
3b359ff
to
e3e4ddb
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #5684 +/- ##
===========================================
+ Coverage 33.77% 39.27% +5.50%
===========================================
Files 351 290 -61
Lines 21832 24486 +2654
Branches 877 0 -877
===========================================
+ Hits 7373 9618 +2245
- Misses 13903 13970 +67
- Partials 556 898 +342
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Yep looks fine to me given a separate story to increase the retry delay after getting errors. It used to break in that case so this is an improvement. :)
This PR has been added to the merge queue, and will be merged soon. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |
1 similar comment
This PR is next in line to be merged, and will be merged as soon as checks pass. |
Description
This PR adds a global rate limit to outgoing P2P unsafe sync requests in addition to the existing local rate limit. The global rate limit outgoing & ingoing rate limits have the same parameters.
This PR also modifies the rate limit parameters. I have increased the burst values (the size of a full token bucket) and in the case of the per peer rate limit decreased the rate. This was done because the burst values were smaller than the per second limits.
Lastly,
clientErrRateCost
must be the same as the per peer rate limit burst because if you request more tokens than the burst, the request will be rejected & the code will not achieve it's goal of delaying future requests to a peer where the request failed.Tests
No tests.
Metadata