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

Adding support for leaky bucket throttling (rps + burst) in ffresty client #141

Merged
merged 11 commits into from
Jun 7, 2024

Conversation

Chengxuan
Copy link
Contributor

@Chengxuan Chengxuan commented May 30, 2024

Adding throttle section as a configuration so the ffresty client can submit requests at certain rates using leaky bucket style throttling control.

Note: this option is not optimal if other clients are using the REST endpoint under the same rate limit bucket.

To handle scenarios like that, we should update ffresty client to be Retry-after header aware or allow users to provide custom handling logic if they use different standards.

@Chengxuan Chengxuan changed the title Adding a common before request rate limiter for RPS Adding support for PRS limit in ffresty client May 30, 2024
Signed-off-by: Chengxuan Xing <[email protected]>
Signed-off-by: Chengxuan Xing <[email protected]>
pkg/ffresty/config.go Outdated Show resolved Hide resolved
pkg/ffresty/ffresty.go Outdated Show resolved Hide resolved
@Chengxuan
Copy link
Contributor Author

@EnriqueL8 thanks for the review. Replied to your comments

Signed-off-by: Chengxuan Xing <[email protected]>
Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan Chengxuan requested a review from EnriqueL8 June 5, 2024 15:07
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

Really great that there's a leaky bucket algorithm built into Go to plug in here 🎉

One request on the configuration split.

Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan
Copy link
Contributor Author

@peterbroadhurst comments addressed

Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan Chengxuan changed the title Adding support for PRS limit in ffresty client Adding support for leaky bucket throttling in ffresty client Jun 6, 2024
@Chengxuan Chengxuan changed the title Adding support for leaky bucket throttling in ffresty client Adding support for leaky bucket throttling (rps + burst) in ffresty client Jun 6, 2024
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

Thanks @Chengxuan for the extra spelling round there - looks great

@peterbroadhurst peterbroadhurst dismissed EnriqueL8’s stale review June 7, 2024 17:08

Now addressed in latest round

@peterbroadhurst peterbroadhurst merged commit c214085 into hyperledger:main Jun 7, 2024
2 checks passed
@peterbroadhurst peterbroadhurst deleted the rps-config branch June 7, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants