Copilot commented on code in PR #13720:
URL: https://github.com/apache/apisix/pull/13720#discussion_r3717952812
##########
apisix/control/router.lua:
##########
@@ -199,9 +199,18 @@ end
end -- do
-local function reload_plugins()
+local function reload_plugins(data, event, source, wid)
+ if wid == ngx.worker.id() then
+ -- already reloaded synchronously in post_reload_plugins()
+ return
+ end
Review Comment:
The worker-events callback’s 4th argument is the publisher process PID, not
the numeric worker id. With the current comparison to `ngx.worker.id()`, the
publishing worker won’t be skipped and will run `plugin_mod.load()` twice per
reload (once synchronously in `post_reload_plugins()`, then again on its own
broadcast). Compare against `ngx.worker.pid()` instead (and rename the
parameter) to correctly skip only the publishing process.
##########
apisix/admin/init.lua:
##########
@@ -408,15 +435,43 @@ local function reload_plugins(data, event, source, pid)
ver = plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY)
end
- plugin.load()
+ local ok, err = plugin.load()
+ -- record the sampled version even when the load failed: the reconciliation
+ -- timer would otherwise retry the very same plugin set every second, and
+ -- every attempt tears the live set down and rebuilds it. The cost is that
+ -- this worker keeps its previous set until some later reload bumps the
+ -- version again -- a load that failed for a transient reason (reading the
+ -- plugin file while it was still being written, say) does not recover on
+ -- its own, it only logs
if ver then
applied_plugins_conf_version = ver
end
+ if not ok then
+ return nil, err
+ end
+
if ngx_worker_id() == 0 then
sync_local_conf_to_etcd()
end
+
+ return true
+end
+
+
+local function reload_plugins(data, event, source, wid)
+ if wid == ngx_worker_id() then
+ -- this worker has already reloaded synchronously while serving the
+ -- Admin API request, see post_reload_plugins()
+ return
+ end
Review Comment:
The worker-events callback’s 4th argument is the publisher process PID (see
other handlers that name it `pid` and log it as such), but this handler treats
it as a worker id. As a result, the publishing worker will not be skipped and
will reload twice (once synchronously in `post_reload_plugins()` and again when
it receives its own broadcast). Compare against `ngx.worker.pid()` (and rename
the parameter) to skip only the publishing process.
--
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]
