From 93a34c2ec1f8add88e5cdd5d4a2bb75359727757 Mon Sep 17 00:00:00 2001 From: chenlin <19296967+lordcheng10@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:02:52 +0800 Subject: [PATCH] fix(limit-count): keep delayed sync counters per rule With multiple rules and sync_interval, the delayed syncer id was derived only from the plugin instance, so all rules shared one queue and one timer. The timer flushes every queued key through the limiter of the syncer that started it, so keys belonging to other rules were synced using the wrong count and time_window, and their Remaining header was overwritten with the first rule's value after each sync. Give each rule its own syncer id, so every rule has its own queue, timer and limiter. Add a regression test with two rules (count 5/100) that checks the remaining headers before and after a delayed sync. --- apisix/plugins/limit-count/init.lua | 8 +- t/plugin/limit-count-redis-delayed-sync2.t | 134 +++++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index d64d8245443f..721586252669 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -425,6 +425,7 @@ local function get_rules(ctx, conf) end return { { + idx = 1, count = count, time_window = time_window, key = conf.key, @@ -451,6 +452,7 @@ local function get_rules(ctx, conf) return nil, err2 end core.table.insert(rules, { + idx = index, count = count, time_window = time_window, key_type = "constant", @@ -603,7 +605,11 @@ local function run_rate_limit(conf, rule, ctx, name, cost, dry_run) lim:log_phase_incoming(key, commit_cost) return elseif enable_delayed_sync then - local extra_key = name .. '#' .. conf.policy + -- the delayed syncer flushes every queued key with its own limiter, + -- so each rule needs a separate syncer (queue and timer). Sharing one + -- per plugin instance would sync the keys of other rules through the + -- count/time_window of whichever rule started the timer. + local extra_key = name .. '#' .. conf.policy .. '#' .. rule.idx if conf._vid then extra_key = extra_key .. '#' .. conf._vid end diff --git a/t/plugin/limit-count-redis-delayed-sync2.t b/t/plugin/limit-count-redis-delayed-sync2.t index 4777f33c910e..06f51c640f23 100644 --- a/t/plugin/limit-count-redis-delayed-sync2.t +++ b/t/plugin/limit-count-redis-delayed-sync2.t @@ -271,3 +271,137 @@ Host: example-7.com "X-RateLimit-Remaining: 0", "X-RateLimit-Remaining: 0", ] + + + +=== TEST 10: set route with multiple rules and delayed sync +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin") + local code, body = t.test('/apisix/admin/routes/echo-multi', + ngx.HTTP_PUT, + [[{ + "uri": "/echo", + "plugins": { + "limit-count": { + "policy": "redis", + "redis_host": "127.0.0.1", + "redis_port": 6379, + "redis_database": 2, + "sync_interval": 0.5, + "show_limit_quota_header": true, + "rejected_code": 503, + "rules": [ + { + "count": 5, + "time_window": 10, + "key": "${remote_addr}_per_minute", + "header_prefix": "rule1" + }, + { + "count": 100, + "time_window": 20, + "key": "${remote_addr}_per_hour", + "header_prefix": "rule2" + } + ] + } + }, + "host": "example-10.com", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 11: each rule keeps its own remaining after delayed sync +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local httpc = http.new() + + local function request() + local res = httpc:request_uri( + "http://127.0.0.1:" .. ngx.var.server_port .. "/echo", { + headers = { + Host = "example-10.com", + }, + }) + if not res then + return nil, "no response" + end + return res + end + + local function header(res, name) + for k, v in pairs(res.headers) do + if k:lower() == name then + return tonumber(v) + end + end + return nil + end + + local res, err = request() + if not res then + ngx.say("FAIL: request before sync: ", err) + return + end + + local r1 = header(res, "x-rule1-ratelimit-remaining") + local r2 = header(res, "x-rule2-ratelimit-remaining") + if not r1 or not r2 then + ngx.say("FAIL: missing remaining headers before sync") + return + end + + if r1 ~= 4 or r2 ~= 99 then + ngx.say("FAIL: unexpected remaining before sync, rule1=", r1, + ", rule2=", r2) + return + end + + -- wait for the delayed sync timer to flush the local deltas to Redis + ngx.sleep(1) + + res, err = request() + if not res then + ngx.say("FAIL: request after sync: ", err) + return + end + + r1 = header(res, "x-rule1-ratelimit-remaining") + r2 = header(res, "x-rule2-ratelimit-remaining") + if not r1 or not r2 then + ngx.say("FAIL: missing remaining headers after sync") + return + end + + -- the second rule must not be synced through the first rule's + -- count/time_window + if r1 ~= 3 or r2 ~= 98 then + ngx.say("FAIL: rule counters corrupted after sync, rule1=", r1, + ", rule2=", r2) + return + end + + ngx.say("PASS") + } + } +--- response_body +PASS