feat(ghttp/middleware): Add memory and redis based token bucket speed limiter #4350
Conversation
LanceAdd
commented
Jul 30, 2025
- 新增基于内存的令牌桶限速器
- 新增基于redis的令牌桶限速器
…mory token bucket algorithm - Add RedisMemoryTokenBucketRateLimiter struct that combines both Redis and Memory rate limiting approaches - Implement AllowN and Middleware methods for RedisMemoryTokenBucketRateLimiter - Add factory methods like NewRedisMemoryTokenBucketRateLimiter - Refactor RedisTokenBucketRateLimiter's AllowN method to support error returning - Optimize logging approach in MemoryTokenBucketRateLimiter
…miter - Solved the accuracy problem that may be caused by floating-point operations to ensure the accuracy of current limiting
…et struct to replace the map, reducing serialization and deserialization to improve performance
- Add unit test file for the glimiter package - Test functionality of both memory-based and Redis-based rate limiters - Verify the Allow and AllowN methods of the rate limiters - Test the creation of rate limiter middleware
There was a problem hiding this comment.
Pull Request Overview
This PR introduces rate limiting functionality for HTTP requests using token bucket algorithms with both memory and Redis-based implementations.
- New glimiter package providing memory-based and Redis-based token bucket rate limiters
- Hybrid Redis+Memory rate limiter with automatic fallback when Redis is unavailable
- HTTP middleware integration for easy adoption in web applications
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| contrib/glimiter/go.mod | New module definition with dependencies for Redis and core GoFrame libraries |
| contrib/glimiter/glimiter_define.go | Common constants, default handlers, and key generation functions |
| contrib/glimiter/glimiter_memory_token_bucket_rate.go | Memory-based token bucket rate limiter with sharded locking for concurrency |
| contrib/glimiter/glimiter_redis_token_bucket_rate.go | Redis-based token bucket rate limiter using Lua scripts for atomic operations |
| contrib/glimiter/glimiter_redis_memory_token_bucket_rate.go | Hybrid limiter combining Redis and memory with automatic Redis fallback |
| contrib/glimiter/glimiter_test.go | Test setup and Redis configuration |
| contrib/glimiter/glimiter_memory_z_unit_test.go | Unit tests for memory-based rate limiter |
| contrib/glimiter/glimiter_redis_z_unit_test.go | Unit tests for Redis-based rate limiter |
| contrib/glimiter/glimiter_memory_redis_z_unit_test.go | Unit tests for hybrid Redis+Memory rate limiter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- 将 shards 变量初始值设为 DefaultShards，默认情况下使用默认分片数- 修改判断条件，当 option.Shards 大于 0 时才使用配置的分片数
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func (l *RedisTokenBucketRateLimiter) AllowN(ctx context.Context, key string, n int64) bool { | ||
| res, err := l.AllowNWithError(ctx, key, n) | ||
| if err != nil { | ||
| l.option.Logger.Errorf(ctx, "[Redis Token Bucket Rate limiter] redis eval error: %+v", err) | ||
| } | ||
| return res |
There was a problem hiding this comment.
Logger may be nil, causing a panic on errors. Initialize a default logger in the constructor when option.Logger is nil.
| redis.call('EXPIRE', key, expire) | ||
| return 1 | ||
| else | ||
| if deny_update == 1 then | ||
| redis.call('HSET', key, 'last_time', last_time) | ||
| redis.call('EXPIRE', key, expire) |
There was a problem hiding this comment.
Using EXPIRE with expire derived from time.Duration.Seconds() truncates sub-second durations to 0, potentially deleting the key immediately. Use millisecond precision with PEXPIRE and pass time.Now().UnixMilli() and option.Expire.Milliseconds().
| redis.call('EXPIRE', key, expire) | |
| return 1 | |
| else | |
| if deny_update == 1 then | |
| redis.call('HSET', key, 'last_time', last_time) | |
| redis.call('EXPIRE', key, expire) | |
| redis.call('PEXPIRE', key, expire) | |
| return 1 | |
| else | |
| if deny_update == 1 then | |
| redis.call('HSET', key, 'last_time', last_time) | |
| redis.call('PEXPIRE', key, expire) |
| incr := delta.Nanoseconds() * m.option.Rate / 1e9 | ||
| tokens += incr | ||
| if tokens > m.option.Capacity { | ||
| tokens = m.option.Capacity | ||
| } |
There was a problem hiding this comment.
delta.Nanoseconds() * m.option.Rate can overflow int64 if lastTime is stale and/or rate is large. Compute increment using float64 (e.g., delta.Seconds()) and clamp before converting back to int64.
| if !l.MemoryTokenBucketRateLimiter.AllowN(ctx, key, 1) { | ||
| l.RedisTokenBucketRateLimiter.option.DenyHandler(r) | ||
| } else { | ||
| l.MemoryTokenBucketRateLimiter.option.AllowHandler(r) |
There was a problem hiding this comment.
[nitpick] Fallback path uses Redis DenyHandler but Memory AllowHandler, which is asymmetric and can confuse consumers. Use a single source of truth for handlers (e.g., always use Redis option handlers) or add explicit top-level handlers on the hybrid option.
| l.MemoryTokenBucketRateLimiter.option.AllowHandler(r) | |
| l.RedisTokenBucketRateLimiter.option.AllowHandler(r) |
| config = &gredis.Config{ | ||
| Address: `:6379`, | ||
| Db: 1, | ||
| } |
There was a problem hiding this comment.
Tests require a real Redis on :6379 DB 1, which can make CI flaky. Consider gating Redis tests behind an env var/build tag, using a testcontainer, or skipping if Redis is unavailable.
| _, err := h.Write([]byte(key)) | ||
| if err != nil { | ||
| m.option.Logger.Errorf(ctx, "[Token Bucket Rate limiter] hash [%s]error: %+v", key, err) | ||
| hash = 0 | ||
| } else { | ||
| hash = h.Sum64() | ||
| } |
There was a problem hiding this comment.
hash.Hash.Write never returns a non-nil error; the error handling here adds noise. You can remove the error branch and compute the hash directly.
| _, err := h.Write([]byte(key)) | |
| if err != nil { | |
| m.option.Logger.Errorf(ctx, "[Token Bucket Rate limiter] hash [%s]error: %+v", key, err) | |
| hash = 0 | |
| } else { | |
| hash = h.Sum64() | |
| } | |
| h.Write([]byte(key)) | |
| hash = h.Sum64() |