sihyeonn commented on PR #12756: URL: https://github.com/apache/apisix/pull/12756#issuecomment-4495083999
Hi @moonming, thank you for the careful review. Your concerns make sense, especially around Redis overhead and memory usage. Let me clarify the intended trade-offs. For the exact `sliding` mode, yes, it is more expensive than the current fixed-window implementation. It uses a Redis sorted set and stores request events within the active window, so both Redis CPU and memory usage increase with traffic. I think this mode is useful when users need stricter rate-limit accuracy near window boundaries, but it is not the mode I would recommend for very high-throughput traffic by default. That is why this PR also includes `approximate_sliding`. This implementation is the same approach we already use internally in production. It keeps counters for the current and previous windows instead of storing every request timestamp, so the memory profile is much closer to fixed window while still smoothing the boundary burst issue that fixed windows have. On the specific points: 1. Redis overhead The Redis operations are wrapped in Lua scripts, so each check/update is a single atomic Redis script execution from the APISIX side. For `sliding`, the script does the sorted-set cleanup, count, insert, expire, and reset calculation. For `approximate_sliding`, the script reads the current and previous counters, calculates the weighted count, increments the current counter, and updates expiration. This is the path we use for production traffic internally. 2. Memory usage For exact `sliding`, memory usage is proportional to the number of request events kept in the active window: ```text number_of_limited_keys * requests_per_key_within_window ``` So for a single hot key at `10K RPS` with a `60s` window, it can keep up to `600K` sorted-set entries. That is the main trade-off of exact sliding window. For `approximate_sliding`, it only keeps counter keys for the current and previous windows, so the memory usage is much closer to the existing fixed-window mode. 3. Atomicity Yes, the Redis operations are atomic because they are executed inside Lua scripts. 4. Migration path Existing users are not affected. The default remains fixed window. Users can opt in with a config-only change: ```json "window_type": "sliding" ``` or: ```json "window_type": "approximate_sliding" ``` I will also add benchmark results comparing `fixed`, `sliding`, and `approximate_sliding` for Redis and Redis Cluster before asking for another review. -- 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]
