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]
