AlinsRan commented on PR #13029: URL: https://github.com/apache/apisix/pull/13029#issuecomment-4849726343
Thanks for tackling this — the stale-IP-after-scale-down problem (#12803) is real. I've been working the same area in #13629 and wanted to flag the overlap, since I think that PR covers this case with fewer trade-offs. **Root of the symptom**: on a node change, dropped targets are marked via `delayed_clear()` and only removed by the periodic cleanup, which had a multi-checker bug ([#13385](https://github.com/apache/apisix/issues/13385)) — so with active health checks the stale nodes lingered. This PR sidesteps that by clearing synchronously. **How #13629 handles it**: for a node-only change (checks unchanged — exactly the k8s/DNS scale-down case), it reconciles the checker's targets in place — `remove_target()` for dropped nodes (synchronous, so the stale IP disappears immediately, same effect as `clear()`) and `add_target()` for new ones — instead of destroying and rebuilding the checker. **Why in-place reconcile is preferable to `clear()` + rebuild:** - Swapping `delayed_clear()` for `clear()` keeps the destroy-then-rebuild path, so the rebuild window ([#13282](https://github.com/apache/apisix/issues/13282)) stays — and immediate `clear()` makes it worse: the shared target list is empty during the rebuild, so `get_target_status()` returns `target not found` and nodes are treated as usable until the new checker re-probes them. - `clear()` wipes *all* targets on *any* node change, so surviving nodes lose their accumulated health state and are re-probed from scratch — heavy churn under frequent k8s/DNS updates. Incremental reconcile only touches the delta. The library-side cleanup bug is fixed separately in api7/lua-resty-healthcheck#55 (shipped via the rockspec bump in #13627), so the `delayed_clear()` path is reliable again as well. Since both PRs modify the same lines in `apisix/healthcheck_manager.lua` (and `t/node/healthcheck-leak-bugfix.t`), they'll conflict. Given #13629 covers this scenario without reintroducing the rebuild window or the state churn, would you be open to closing this in favor of #13629? I'll make sure the k8s scale-down case is explicitly covered there. Thanks again for surfacing it! -- 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]
