Copilot commented on code in PR #14015:
URL: https://github.com/apache/apisix/pull/14015#discussion_r4216482825
##########
apisix/plugins/limit-count/delayed-syncer.lua:
##########
@@ -340,15 +340,21 @@ local function sync_key(self, key)
core.log.error("get local delta from shm failed: ", err)
end
- if delta then
+ -- the key expires two windows after its creation and is not renewed, so a
+ -- node that only rejects loses it; flush anyway, otherwise that node keeps
+ -- judging on the same cached quota until the quota itself expires
+ if not err then
+ local had_delta = delta ~= nil
Review Comment:
After the delta expires, a successful zero-delta refresh calls
`sync_to_shm`, whose `incr(..., 0, 0, ...)` recreates the entry with value 0.
If Redis then becomes unavailable before another request is admitted,
`had_delta` is true even though there is nothing to account for. The fallback
path charges 1 and `sync_to_shm` subtracts it from 0, leaving a negative delta
that the sliding-window backend can later commit to Redis, reducing the remote
count. Require a positive pending delta before charging fallback so the new
refresh-only path cannot cause this accounting error.
--
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]
