AlinsRan commented on PR #13629:
URL: https://github.com/apache/apisix/pull/13629#issuecomment-4852261946

   Good catch — confirmed. `fetch_checker()` gated on an exact `version` match, 
so on a node-only `_nodes_ver` bump it returned `nil` (only enqueuing the new 
version) until the 1s timer ran; `upstream.lua` then set `api_ctx.up_checker = 
nil` and `balancer.lua` built the picker from all nodes. The health state was 
preserved internally but not used during the window — exactly as you described.
   
   Fixed in 3688210f3: `fetch_checker()` now returns the existing **live** 
(non-dead) checker for the resource while it enqueues the new version for 
reconciliation, so requests keep filtering unhealthy nodes throughout the 
transition.
   
   One deviation from your suggestion worth flagging: I return the live checker 
whenever one exists (and isn't dead), rather than gating on reuse-eligibility 
(`checks` unchanged + nodes non-empty). That guard would need a 
`fetch_latest_conf()` + `deep_eq` of the checks on the **per-request hot 
path**. Returning the live checker is safe even in the rarer checks-change 
window — its health status is real and current for the pre-change checks, which 
is strictly better than `nil`/no-filtering — and once the timer publishes the 
rebuilt checker the exact-version branch takes over. Happy to add the 
eligibility guard if you'd prefer it despite the hot-path cost.
   
   Regression test (`healthcheck-incremental-update.t` TEST 4): mark a node 
unhealthy via active checks, apply a node-only update, then burst requests in 
the transition window and assert none hit the unhealthy node. Verified locally 
— it **fails before** the fix (`errors: 3`, the dead node is picked during the 
window) and **passes after** (`errors: 0`).
   


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