AlinsRan commented on PR #13443: URL: https://github.com/apache/apisix/pull/13443#issuecomment-4665569401
Follow-up: the latest commit 17a6117 fixes a global under-counting bug that affected the **sliding-window + delayed-sync** combination. ### Problem In delayed-sync mode, requests are admitted locally and the accumulated delta is later flushed to the shared store. The flush reused the sliding-window `incoming()` path, but `incoming()` rejects **before** incrementing once the counter is at/over the limit: ``` incoming(): if over limit -> return "rejected" (no store:incr) ``` So when a node had already locally permitted requests that pushed the window over the limit, the flush of that already-consumed delta was silently dropped instead of written to Redis. Those requests really happened, so the shared counter permanently under-records them, and the cluster-wide quota drifts above the configured limit over time. This is the same reason the fixed-window backend never had the bug: its Redis script *always* increments first and only then decides whether to report `rejected`, so a delayed-sync flush is never lost. ### Fix Add a `commit()` method to the sliding-window backend that unconditionally increments the counter and reports the resulting remaining, skipping the pre-increment rejection. The delayed syncer now prefers it when available: ```lua local flush = self.limiter.commit or self.limiter.incoming ``` `commit()` mirrors the fixed-window semantics: an already-permitted delta always reaches the shared store, even when the window is already over the limit. Backends without a `commit()` fall back to `incoming()`, so behavior is unchanged for them. Regression test added in `t/plugin/limit-count-sliding.t`: fill the window via `incoming()`, confirm a further `incoming()` is rejected with no increment, then confirm `commit()` still flushes the delta and drives `remaining` negative as expected. -- 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]
