nic-6443 opened a new pull request, #13892:
URL: https://github.com/apache/apisix/pull/13892

   ### Description
   
   `t/lib/test_redis.lua`'s `wait_counters_above()` gave up after one second, 
and one second is not always enough for the counter it waits on to show up. 
This raises the bound to five seconds.
   
   Some context on why the wait exists at all, since that is where the 
flakiness comes from. `ai-rate-limiting` commits token usage in its `log` 
phase, which APISIX runs from `log_by_lua_block` — after the response has 
already gone out to the client. Cosockets are not available there, so 
`limit-count-redis`'s `log_phase_incoming()` defers the actual write with 
`ngx.timer.at(0, ...)`. That means when the test client holds request N's 
response, the counter for N is still unwritten: the log phase has to run, the 
zero-delay timer has to be picked up by the event loop, and the redis roundtrip 
has to complete. Fire request N+1 immediately and its access phase reads a 
budget that still looks unspent — which is the off-by-one-request symptom these 
tests used to show, wrong status codes and `X-AI-RateLimit-Remaining` values 
lagging by one request. `wait_counters_above()` bridges that gap: snapshot with 
`sum_counters()` before the request, then poll until the total grows.
   
   The flaky part is the ceiling on that poll, not the logic. 100 iterations of 
`ngx.sleep(0.01)` is one second of wall clock, and that one second has to cover 
all three hops above plus the poller's own `KEYS` + `GET` against the same 
redis. On a loaded runner — parallel jobs sharing one redis container, a worker 
that doesn't get scheduled promptly — the write is still in flight when the 
poll runs out, so the helper returns an error and the caller's `assert` fails 
even though the counter lands a moment later. Nothing about it is 
deterministic, which is why it turns up on PRs that touch nothing in this area.
   
   500 iterations gives it five seconds. The loop still returns as soon as the 
counter moves, so a healthy run costs exactly what it did before — only the 
ceiling changes.
   
   One consequence worth knowing: the caller, `t/plugin/ai-rate-limiting.t` 
TEST 37, carries `--- timeout: 10` and waits up to three times, so a counter 
that genuinely never lands now trips the test-level timeout rather than 
surfacing the helper's own error message. That only affects the already-failing 
path.
   
   ### Checklist
   
   - [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
   - [ ] 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 (If not, 
please discuss on the [APISIX mailing 
list](https://github.com/apache/apisix/tree/master#community) first)
   
   This is a test-only change to a test helper, so no new tests or 
documentation.
   


-- 
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]

Reply via email to