sihyeonn commented on code in PR #12756:
URL: https://github.com/apache/apisix/pull/12756#discussion_r2601493623
##########
apisix/plugins/limit-count/limit-count-redis-cluster.lua:
##########
@@ -39,7 +40,56 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+ local req_id = ARGV[5]
+
+ local window_start = now - window
+
+ redis.call('ZREMRANGEBYSCORE', KEYS[1], 0, window_start)
+
+ local current = redis.call('ZCARD', KEYS[1])
+
+ if current + cost > limit then
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+ return {-1, reset}
+ end
+
+ for i = 1, cost do
+ local member = req_id .. ':' .. i
+ redis.call('ZADD', KEYS[1], now, member)
+ end
Review Comment:
@nic-6443
Thanks for raising this concern!
In this sliding-window implementation, we intentionally trade some Redis
work per request for *exact* “N requests per rolling window” semantics.
However, the memory and per-key load are bounded:
- on every request we first call `ZREMRANGEBYSCORE` to drop all events
outside of the current window and then enforce `current + cost <= limit` before
inserting new members,
- rejected requests do **not** insert new entries,
- so for each key the ZSET cardinality is always `<= limit`, and Redis
memory for this key is bounded by that limit.
Because of this, even though we do one `ZADD` per allowed request, we don’t
end up with unbounded growth like in some TTL-only designs – the sliding window
state per key stays O(limit). Also, `window_type = "sliding"` is an opt-in mode
and the docs explicitly call out that it is more expensive than the default
fixed window, so it’s intended for use cases that really need accurate rolling
windows and can accept the extra Redis cost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
