Copilot commented on code in PR #13572:
URL: https://github.com/apache/apisix/pull/13572#discussion_r3435386037
##########
apisix/control/v1.lua:
##########
@@ -69,6 +69,54 @@ function _M.schema()
end
local healthcheck
+
+
+-- targets the checker registers for this upstream, keyed host:port
+-- (active-check port overrides the node port)
+local function get_configured_targets(up_conf)
+ local nodes = up_conf and up_conf.nodes
+ if type(nodes) ~= "table" then
+ return nil
+ end
+
+ local active = up_conf.checks and up_conf.checks.active
+ local override_port = active and active.port
+ local targets = core.table.new(0, #nodes)
+
Review Comment:
`core.table.new(0, #nodes)` uses the length operator for preallocation,
which will be 0 when `nodes` is a hash table (the common `{ "ip:port": weight
}` form). This doesn’t break correctness, but it defeats the intended
preallocation and can cause extra rehashing for large upstreams; use
`core.table.nkeys(nodes)` for the map case.
##########
docs/en/latest/control-api.md:
##########
@@ -134,7 +134,7 @@ Each of the returned objects contain the following fields:
* name: resource id, where the health checker is reporting from.
* type: health check type: `["http", "https", "tcp"]`.
-* nodes: target nodes of the health checker.
+* nodes: target nodes of the health checker. Nodes removed from the upstream
are not reported, even if their checker state has not been cleaned up yet.
Review Comment:
This sentence reads like an unconditional guarantee, but the implementation
is explicitly fail-open (it returns the original node list unchanged if the
configured node set can’t be determined, e.g. discovery-based upstreams without
a `nodes` table). Consider softening the wording to reflect that filtering
applies when APISIX can reconcile against the current upstream nodes
configuration.
--
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]
