From db01544e064c0ad79c3759e44cc0cefa7cd6aae6 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 20 Jul 2026 13:12:22 +0800 Subject: [PATCH 1/2] fix(admin): reconcile plugins reload instead of trusting the broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit events:post has no delivery guarantee. A process that is connecting or reconnecting to the events broker when a reload is accepted loses that event for good, and nothing ever brings it back in line: plugin.load() has only two call sites and neither of them reconciles. The visible symptom is the privileged agent still running timers of plugins that were removed — log-rotate keeps rotating long after it was disabled. Bump a version counter in the internal-status shared dict before broadcasting, and have every process that registers the reload handler compare it once a second, loading the plugins again when it differs. The broadcast stays as the sub-second fast path; the counter is the slow path that actually guarantees convergence. This mirrors what admin/standalone.lua already does, for the same reason. plugin.load() is idempotent, so a redundant round is harmless. The version is sampled before loading, so a reload accepted while load() runs leaves the versions unequal and gets applied on the next tick. A fresh worker adopts the current version at init_worker, which keeps a HUP reload from triggering a spurious load. Two gaps are left untouched: the stream subsystem shares neither the shared dict nor the events socket with http, and propagation between instances still goes through etcd. Both are pre-existing. Fixes #13537 --- apisix/admin/init.lua | 57 ++++++++++++++++++ t/admin/plugins-reload-reconcile.t | 92 ++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 t/admin/plugins-reload-reconcile.t diff --git a/apisix/admin/init.lua b/apisix/admin/init.lua index c3f543e62949..f3b4323249d5 100644 --- a/apisix/admin/init.lua +++ b/apisix/admin/init.lua @@ -27,11 +27,20 @@ local ngx = ngx local get_method = ngx.req.get_method local ngx_time = ngx.time local ngx_timer_at = ngx.timer.at +local ngx_timer_every = ngx.timer.every local ngx_worker_id = ngx.worker.id local tonumber = tonumber local tostring = tostring local str_lower = string.lower local reload_event = "/apisix/admin/plugins/reload" +local is_http = ngx.config.subsystem == "http" +-- declared unconditionally for the http subsystem in ngx_tpl.lua, and already a +-- hard dependency of the server-info plugin; absent in the stream subsystem +local plugins_conf_ver_dict = is_http and ngx.shared["internal-status"] +local PLUGINS_CONF_VERSION_KEY = "plugins_conf_version" +-- plugins conf version this process has applied, compared against the shared +-- dict by the reconciliation timer registered in init_worker() +local applied_plugins_conf_version = 0 local ipairs = ipairs local error = error local type = type @@ -288,6 +297,17 @@ end local function post_reload_plugins() set_ctx_and_check_token() + if plugins_conf_ver_dict then + -- bump the version before broadcasting, so that a process which never + -- receives the event (e.g. the privileged agent while it is + -- reconnecting to the events broker) still converges through the + -- periodic reconciliation below + local _, err = plugins_conf_ver_dict:incr(PLUGINS_CONF_VERSION_KEY, 1, 0) + if err then + core.log.error("failed to increase plugins conf version: ", err) + end + end + local success, err = events:post(reload_event, get_method(), ngx_time()) if not success then core.response.exit(503, err) @@ -375,8 +395,21 @@ end local function reload_plugins(data, event, source, pid) core.log.info("start to hot reload plugins") + + -- sample the version before loading: if another reload is accepted while + -- plugin.load() runs, the versions stay unequal and the reconciliation + -- timer applies one more round + local ver + if plugins_conf_ver_dict then + ver = plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) + end + plugin.load() + if ver then + applied_plugins_conf_version = ver + end + if ngx_worker_id() == 0 then sync_local_conf_to_etcd() end @@ -509,6 +542,30 @@ function _M.init_worker() events = require("apisix.events") events:register(reload_plugins, reload_event, "PUT") + if plugins_conf_ver_dict and not is_yaml_config_provider then + -- The events broadcast has no delivery guarantee: a process that is + -- (re)connecting to the events broker loses the event for good, which + -- leaves it running e.g. the timers of plugins that were removed. + -- Reconcile against the version in the shared dict, the same pattern + -- admin/standalone.lua uses for the same reason. + applied_plugins_conf_version = + plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) or 0 + + local ok, err = ngx_timer_every(1, function (premature) + if premature then + return + end + + local ver = plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) or 0 + if ver ~= applied_plugins_conf_version then + reload_plugins() + end + end) + if not ok then + core.log.error("failed to create plugins reconciliation timer: ", err) + end + end + if ngx_worker_id() == 0 then -- check if admin_key is required if local_conf.deployment.admin.admin_key_required == false then diff --git a/t/admin/plugins-reload-reconcile.t b/t/admin/plugins-reload-reconcile.t new file mode 100644 index 000000000000..d8d64363ec55 --- /dev/null +++ b/t/admin/plugins-reload-reconcile.t @@ -0,0 +1,92 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +use t::APISIX 'no_plan'; + +log_level('info'); +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests(); + +__DATA__ + +=== TEST 1: a reload request bumps the shared plugins conf version +--- config + location /t { + content_by_lua_block { + local dict = ngx.shared["internal-status"] + local before = dict:get("plugins_conf_version") or 0 + + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/plugins/reload', ngx.HTTP_PUT) + + local after = dict:get("plugins_conf_version") or 0 + ngx.say(code, " ", body, " bumped=", tostring(after > before)) + } + } +--- response_body +200 passed bumped=true +--- no_error_log +failed to increase plugins conf version + + + +=== TEST 2: a process that missed the broadcast reloads through reconciliation +--- config + location /t { + content_by_lua_block { + -- A reload whose broadcast never arrives leaves the shared version + -- ahead of what this process applied. Reproduce exactly that state + -- without going through the events layer. + local dict = ngx.shared["internal-status"] + dict:incr("plugins_conf_version", 1, 0) + + -- the reconciliation timer runs once a second + ngx.sleep(2.5) + ngx.say("done") + } + } +--- response_body +done +--- error_log +start to hot reload plugins + + + +=== TEST 3: an unchanged version never triggers a reload +--- config + location /t { + content_by_lua_block { + ngx.sleep(3) + ngx.say("done") + } + } +--- response_body +done +--- no_error_log +start to hot reload plugins From 5b2ac7b44099c30e9d0b28441c4b358fcb594265 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Tue, 21 Jul 2026 08:07:33 +0800 Subject: [PATCH 2/2] fix(admin): fail the reload when the version bump fails If the shared-dict incr fails the reconciliation timer never sees a change, so a worker that misses the broadcast stays stale forever. Return 503 instead of logging and reporting success. Also check the incr in the reconcile test so a setup failure surfaces directly. --- apisix/admin/init.lua | 4 ++++ t/admin/plugins-reload-reconcile.t | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/apisix/admin/init.lua b/apisix/admin/init.lua index f3b4323249d5..3ef8fafc8fd0 100644 --- a/apisix/admin/init.lua +++ b/apisix/admin/init.lua @@ -304,7 +304,11 @@ local function post_reload_plugins() -- periodic reconciliation below local _, err = plugins_conf_ver_dict:incr(PLUGINS_CONF_VERSION_KEY, 1, 0) if err then + -- if the version cannot be bumped the reconciliation timer will + -- never notice a change, so a worker that misses the broadcast + -- would stay stale forever; fail loud instead of pretending success core.log.error("failed to increase plugins conf version: ", err) + core.response.exit(503, {error_msg = "failed to record plugins reload"}) end end diff --git a/t/admin/plugins-reload-reconcile.t b/t/admin/plugins-reload-reconcile.t index d8d64363ec55..df7a2a396426 100644 --- a/t/admin/plugins-reload-reconcile.t +++ b/t/admin/plugins-reload-reconcile.t @@ -64,7 +64,11 @@ failed to increase plugins conf version -- ahead of what this process applied. Reproduce exactly that state -- without going through the events layer. local dict = ngx.shared["internal-status"] - dict:incr("plugins_conf_version", 1, 0) + local newver, incr_err = dict:incr("plugins_conf_version", 1, 0) + if not newver then + ngx.say("failed to bump version: ", incr_err) + return + end -- the reconciliation timer runs once a second ngx.sleep(2.5)