nic-6443 opened a new pull request, #14015: URL: https://github.com/apache/apisix/pull/14015
With delayed sync, a node that only rejects can stop refreshing its cached quota, and then it keeps rejecting on stale numbers until the quota itself expires. The periodic sync flushes a key only while its local delta is present in the shared dict (`sync_key`, `if delta then`). That entry is created by `incr(..., 0, 2 * window)`, and `init_ttl` only applies on creation, so the entry expires two windows after it was first written, whatever happens in between. Admitted requests recreate it; rejected ones don't. So once it expires on a node that is rejecting everything, the timer still runs for the key but skips the flush, and the cached quota is never refreshed again. The next refresh only happens when the cached reset runs out. With a sliding window this matters a lot. The cached quota carries the previous window's weight as of the last sync, so the node keeps applying a weight of ~0.9 for the rest of the window instead of letting it decay. In a two-node test at 2× the limit, one 10s window admitted 9 requests where an exact sliding window would have admitted 100, because both nodes stopped syncing about 1s into the window. A fixed window isn't affected in practice: within a window the remote count only grows, so a stale cached quota can't wrongly reject, and the window end coincides with the cached reset, which forces a refresh. The fix flushes a missing delta as 0 instead of skipping it. That refreshes the cached quota exactly like the existing flush of a zero delta, which already happens whenever the entry is present but empty. A failed shared dict read still skips the flush as before. `t/plugin/limit-count-redis-delayed-sync3.t` reproduces it without waiting two windows. It exhausts the quota, drops the local delta entry as its expiry would, frees the quota in Redis, and checks that a rejected request schedules a sync that lets the next request through. It returns `200 200 503 503 503` without the fix and `200 200 503 503 200` with it. No documentation change: the documented delayed sync behavior doesn't change. - [x] I have explained the need for this PR and the problem it solves - [x] I have explained the changes or the new features added to this PR - [x] I have added tests corresponding to this change - [ ] I have updated the documentation to reflect this change - [x] I have verified that this change is backward compatible -- 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]
