fix(standalone): stop aborting stream connections before the first config arrives - #13855
Open
AlinsRan wants to merge 1 commit into
Open
fix(standalone): stop aborting stream connections before the first config arrives#13855AlinsRan wants to merge 1 commit into
AlinsRan wants to merge 1 commit into
Conversation
…nfig arrives
In standalone mode the stream subsystem accepts connections as soon as it
starts, which can be well before any configuration has reached it, and every
one of those connections was aborted with
core/config_util.lua:37: attempt to index local 'tab' (a nil value)
stream/router/ip_port.lua:76: in function 'create_router'
stream/router/ip_port.lua:154: in function 'match'
config_yaml.sync_data leaves `values` nil while `conf_version` is still 0,
because `apisix_yaml[<key>_conf_version] or 0` equals the initial conf_version
and it returns early. `router_ver` starts as nil, so `router_ver ~=
user_routes.conf_version` holds and match() calls create_router(nil). The error
is thrown before router_ver is assigned, so the branch is taken again on the
next connection: the worker never leaves this state on its own.
Treat a missing route list as "no stream route" and let the connection take the
normal no-match path.
The second half is why the worker stayed in that state. Only the polling in
admin/standalone.lua carries configuration across the http/stream boundary, and
it skipped an update whenever X-Last-Modified was unchanged. That field is
ngx_time(), so two updates in the same second are indistinguishable by it: if
the poll ran between them, the second update was dropped for good, and nothing
later re-delivered it because a client re-sending the same content is answered
with "config not changed: same digest". Compare the digest as well, which
changes with the content.
Reproduces without a client: start APISIX in standalone mode with a
stream_proxy port, connect to that port before pushing any configuration, and
the connection is aborted with the trace above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In standalone mode the stream subsystem starts accepting connections before any configuration has reached it, and every connection that arrives in that window is aborted:
The client sees an empty response, not an error, so this shows up as a stream port that silently answers nothing.
Why
valuesis nil.config_yaml.sync_datareturns early while the config source has not delivered anything yet:user_routesstarts as{conf_version = 0, values = nil}, sovaluesstays nil whileconf_versionis 0.Why the connection aborts. In
ip_port.lua,router_verstarts as nil, sorouter_ver ~= user_routes.conf_version(nil ~= 0) holds andcreate_router(user_routes.values)is called with nil. The error is thrown beforerouter_veris assigned, so the next connection takes the same branch and aborts again — the worker does not leave this state on its own.Why it does not recover. Only the polling loop in
admin/standalone.luacarries configuration across the http/stream boundary (the events module cannot broadcast between subsystems, as the comment there says). It skipped an update wheneverX-Last-Modifiedwas unchanged, and that field isngx_time()— second resolution. Two updates inside one second are indistinguishable by it, so if the poll ran between them the second update was dropped permanently: nothing re-delivers it later, because a client re-sending the same content is answered withconfig not changed: same digestand the stored metadata never advances.That combination is what I hit in the field: a control plane pushed several configs inside one second, the first without
stream_routesand a later one with them, and the stream workers of that instance answered nothing for the rest of the pod's life while the HTTP subsystem and the Admin API both showed the stream route present.Fix
apisix/stream/router/ip_port.lua: treat a missing route list as "no stream route" instead of indexing nil, so the connection takes the normal no-match path and the router is rebuilt once the routes arrive.apisix/admin/standalone.lua: compareX-Digestas well asX-Last-Modifiedin the polling loop. The digest changes with the content, so a second update inside the same second is no longer invisible to the other subsystem.Reproduce
No control plane needed:
With this patch the same connection logs
ip_port.lua: match(): not hit any routeand no error, and once a config carryingstream_routesis pushed the connection is routed normally.Checklist
Note on the test:
t/admin/standalone-stream.tasserts that a stream connection made before the first config push takes the no-match path and logs no abort. I could not run test-nginx locally — the openresty on this machine predatesapisix_stream_metrics_zone, so pre-existing stream tests such ast/config-center-yaml/stream-route.tfail to start nginx here as well. The behaviour was verified againstapache/apisix:devwith the two patched files mounted in, as described above.