-
Notifications
You must be signed in to change notification settings - Fork 1.7k
En/rate limiter #2263
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
base: development
Are you sure you want to change the base?
En/rate limiter #2263
Conversation
@Umang01-hash We should consider having support for generic time window, i.e. not limit to per second. Most systems have per minute/hour limits. |
71e1f9b
to
aae16f3
Compare
@akshat-kumar-singhal sure fixed the same Users can use it like:
|
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.
-
Validate Window ≥1 ms
Prevent micro/nanosecond windows that spike CPU and precision issues:
if config.Window<time.Millisecond { return fmt.Errorf("min window 1ms, got %v",config.Window) } -
RPS Calculation
Avoid repeated division: compute once and reuse cachedRPS. -
Preload Lua Script
Load script SHA on init and use EVALSHA to skip re-compiling on every Redis call.
|
||
type Logger interface { | ||
Log(args ...any) | ||
Debug(args ...any) |
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.
Altering an exported interface is a breaking change
httpSvc := h.(*httpService) | ||
|
||
rl := &distributedRateLimiter{ | ||
config: config, |
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.
this config should be validated
|
||
var ( | ||
errInvalidRequestRate = errors.New("requestsPerSecond must be greater than 0") | ||
errInvalidBurstSize = errors.New("burst must be greater than 0") |
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.
Additional validation: Burst size should be greater than requests per unit time
type RateLimiterConfig struct { | ||
Requests float64 // Number of requests allowed | ||
Window time.Duration // Time window (e.g., time.Minute, time.Hour) | ||
Burst int // Maximum burst capacity (must be > 0) |
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.
Burst can be optional - set equal to Requests
in case not configured/zero.
) | ||
|
||
var ( | ||
errInvalidRequestRate = errors.New("requestsPerSecond must be greater than 0") |
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.
Error message should be updated as we are supporting other time windows as well
// distributedRateLimiter with metrics support. | ||
type distributedRateLimiter struct { | ||
config RateLimiterConfig | ||
redisClient *gofrRedis.Redis |
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.
Should this be a "cache interface" or "rate limiter bucket" instance instead of specifically Redis? That way the implementation difference between Redis/Local would just be at the storage level and not in the core functionality
Add Rate Limiting Support for HTTP Services
This PR adds rate limiting capabilities to # Add Rate Limiting Support for HTTP Services
This PR adds rate limiting capabilities to GoFr's HTTP service clients using the token bucket algorithm. It provides both local (in-memory) and distributed (Redis-based) implementations to suit different deployment scenarios.
Features
Two Implementation Strategies:
Core Features:
Usage Example
Usage Screenshots:
Checklist:
goimport
andgolangci-lint
.Thank you for your contribution!