Fix requests per second, proper error responses, compilation related to the curl-sys library #3405
Conversation
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
|
Thx! Mind removing the MSRV bump? It'll need to be updated in Also perhaps someone will object using the latest MSRV, so a separate PR is a better ground for discussion. Will also ask some people directly about their opinion. |
|
Sure, removed it. I couldn't get it to compile without updating the rust version, so you may need to update that before this is merged in |
evanmarshall
approved these changes
Oct 17, 2024
|
Can you merge |
Signed-off-by: Mike <[email protected]>
done -- feel free to change this anyway you all need to get it merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Fixes #3402. This PR includes a patch for #3403, in order to get this code to compile.
Test Plan
Manually tested with low rps, verified that the requests per second did not become 1 after the burst quota was exhausted.
Additional Details
This is our patch fix, so feel free to adapt this code as necessary. We use
per_nanosecondinstead ofper_secondfor the replenishment rate, with the goal of replenishing the burst rate in full every second. Our primary concern was the rps, which is why the 429 (Too many requests) error in particular is handled but other errors are defaulted to a 500 (Internal Server Error). Furthermore, related to issue #3403, I had to bump the rust version in order to get snarkos to compile.