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.

Sign up for GitHub

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

Jump to bottom

Caplin: Add a request limiter #8868

Merged
merged 7 commits into from
Dec 1, 2023
Merged

Caplin: Add a request limiter #8868

merged 7 commits into from
Dec 1, 2023

Conversation

bayram98
Copy link
Contributor

@bayram98 bayram98 commented Nov 30, 2023 •
edited
Loading

This PR is to add the request rate limiter.

The solution is to count the request number for each peer for each minute, if the peer exceeds the limit, block the requests for a specified time.

Current limits:

  • Request limited to 5000 requests per minute for each handler.
  • Penalty blockage time 1-minute

@bayram98 bayram98 self-assigned this Nov 30, 2023
@bayram98 bayram98 linked an issue Nov 30, 2023 that may be closed by this pull request
Caplin: Add a bandwidth limiter #8723
Closed
@bayram98 bayram98 changed the title add cl request limiter Caplin: Add a request limiter Nov 30, 2023
Giulio2002
Giulio2002 requested changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@Giulio2002 Giulio2002 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just few adjustments needed

cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
cl/sentinel/handlers/heartbeats.go Show resolved Hide resolved
cl/sentinel/handlers/heartbeats.go Show resolved Hide resolved
cl/sentinel/handlers/heartbeats.go Show resolved Hide resolved
cl/sentinel/handlers/heartbeats.go Show resolved Hide resolved
cl/sentinel/handlers/heartbeats.go Show resolved Hide resolved
@elee1766
Copy link
Contributor

I would replace ratelimiter with this

https://pkg.go.dev/golang.org/x/time/rate

@Giulio2002
Copy link
Contributor

I would replace ratelimiter with this

https://pkg.go.dev/golang.org/x/time/rate

Agree, that would get rid of most of the responsibility on us to mantain.

@bayram98 bayram98 marked this pull request as ready for review December 1, 2023 12:14
Giulio2002
Giulio2002 approved these changes Dec 1, 2023
View reviewed changes
cl/sentinel/handlers/handlers.go Outdated Show resolved Hide resolved
@Giulio2002 Giulio2002 merged commit e7e1bf4 into devel Dec 1, 2023
6 checks passed
@Giulio2002 Giulio2002 deleted the feat/add-limitcheck-to-cl branch December 1, 2023 19:11
@BrewTestBot BrewTestBot mentioned this pull request Dec 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Giulio2002 Giulio2002 Giulio2002 approved these changes

@elee1766 elee1766 Awaiting requested review from elee1766

Assignees

@bayram98 bayram98

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Caplin: Add a bandwidth limiter
3 participants
@bayram98 @elee1766 @Giulio2002