fix(plugin): make the hot reload of plugins transactional

[AlinsRan]

Description

Fixes #13087.

plugin.load() tears the live plugin tables down before rebuilding them: it destroys every old plugin, clears local_plugins / local_plugins_hash in place, then re-requires and re-init()s the plugins one by one, with no pcall and no rollback.

If one init() throws, the worker is left permanently broken:

• load_plugin() inserts into the array before calling init(), so even the failing plugin stays in the served array;
• the local_plugins_hash rebuild loop never runs, so the hash stays empty and the Admin API rejects every plugin with unknown plugin [...];
• which plugins survive depends on the pairs() hash order, so it differs per worker;
• the error is swallowed by the worker-events pcall, and the reload endpoint has already answered 200 done.

Reproduced on master: after a reload with one plugin whose init() throws, plugin.plugins = [bad-init], plugin.plugins_hash = {}, the previously working route silently loses its response-rewrite, and re-PUTing the exact same route conf returns 400 unknown plugin [response-rewrite]. The state is permanent until the next successful reload.

The same structure is also what makes the second half of the issue possible: the live tables are cleared and then repopulated across require() / init() calls, which can yield, so an in-flight request can traverse a partial plugin set.

Solution

load() / load_stream() become a three-phase transaction:

1. build — the new plugin set is built in a local table. require() and the completeness checks all happen here; the tables the request path reads are untouched, so nothing is observable yet.
2. switch hooks — destroy the old instances, then run init() / workflow_handler() of the new instances, all under pcall. If any hook fails: destroy the new instances that were initialized, restore the package.loaded snapshot, re-init() the old instances, and return the error. The live tables were never touched.
3. commit — repopulate the live tables in place.

Three deliberate choices that are not the obvious ones:

• Old instances are destroyed before the new ones are initialized, not after. Several in-tree plugins register global resources by name: server-info, log-rotate and error-log-logger call timers.register_timer("plugin#<name>", ...) in init() and timers.unregister_timer(...) in destroy(). If the new instance registered first and the old one was destroyed afterwards, the old destroy() would unregister the timer the new instance had just registered — a "successful" reload that silently loses background tasks. Keeping today's order and implementing the rollback as "re-init() the old instances" avoids that; init/destroy are already written to be re-runnable across reloads.
• The rollback destroys only the instances whose init() actually ran, in the reverse order of init(). destroy() of an instance that was never initialized publishes its uninitialized state: gm and ocsp-stapling assign their still-nil upvalue back to radixtree_sni.set_cert_and_key (and apisix_ssl.check_ssl_conf), which breaks the SSL handshake path — and the rollback then saves that nil as the "original" function when it re-init()s the old instance, so the damage outlives the reload. Both plugins have a lower priority than almost anything else (-43 / -44), so they sit at the end of the array and any failing init() reaches them. The reverse order matters for the same pair: gm (priority -43) installs first and is the inner layer, ocsp-stapling (-44) wraps it, so the wrappers must be unwound LIFO. With the previous forward destroy, even a successful reload left set_cert_and_key pointing at the stale old-gm wrapper — gm.destroy() restored the real function and ocsp.destroy() then overwrote it with gm's wrapper — permanently leaking the old module into the call chain. The old instances are destroyed in reverse order too, for that reason.
• The live tables are repopulated in place, not swapped. _M.plugins is bound to local_plugins at module load time, and apisix/api_router.lua and apisix/control/router.lua read it via ipairs(plugin_mod.plugins); swapping the upvalue would leave those readers holding a dead table. In-place repopulation keeps the table identity, and there is no yield point between the clear and the end of the loop, so a concurrent request cannot observe a half-built set. ctx.plugins holds plugin object references, so in-flight requests finish against the old objects exactly as they do today.

init() moves out of load_plugin() into the caller, which is what makes "insert before init" stop mattering.

require / priority / version / schema failures keep today's "log and skip" semantics — that is also the cold-start behaviour, and such a failure does not corrupt anything (the plugin is simply absent, array and hash stay consistent). Only init / workflow_handler failures, which have side effects, abort the reload. Making the reload stricter about unknown plugin names is a separate discussion.

Behaviour change (please read)

This changes the public behaviour of the reload endpoints:

• PUT /apisix/admin/plugins/reload and PUT /v1/plugins/reload now perform the load synchronously on the serving worker first, and only broadcast the event to the other workers if it succeeded.
• On failure they return 500 with the error message instead of 200 done, and no event is broadcast, so the other workers are not asked to break themselves too.
• The success path is unchanged: still 200 done.
• The endpoints are now slower on the success path, since the request does a full load (require + init hooks) instead of only posting an event. Reload is a low-frequency operational call, so this seemed the right trade.

I think the current 200 done is the worse behaviour: the operator is told the reload succeeded while the plugin table is corrupted and the Admin API has started rejecting every plugin. But this will be visible to anyone whose automation asserts "reload always returns 200", so it deserves discussion rather than a silent merge.

The event handlers skip the originating worker (via the worker id the events frame carries) so the serving worker does not load twice.

Rebased on the reconciliation work that landed meanwhile (#13714, #13745): the version is now bumped after the synchronous load succeeded, not before, so a plugin set which cannot load neither reaches the other workers nor arms the reconciliation timer — otherwise every worker would retry the same broken set once a second, and each attempt tears the live plugin set down and rebuilds it. For the same reason reload_plugins_and_sync() records the sampled version even when the load failed. The serving worker also stores the version it just bumped, so its own timer does not redo the load one second later.

Tests

New t/admin/plugins-reload-transaction.t, plus test-only fixtures: t/apisix/plugins/reload-bad-init.lua, whose init() always throws, and t/apisix/plugins/reload-probe.lua / reload-probe-2.lua, which record their lifecycle hooks into t/lib/reload_probe_state.lua (kept outside the apisix.plugins package so the loader never drops it from package.loaded, hence the counters survive a reload). Both are resolved through the existing $apisix_home/t/?.lua package path, so they do not ship in the release.

• TEST 1 reloads a plugin list that keeps response-rewrite and adds the throwing plugin, then asserts: the endpoint returns 500 with failed to init plugin [reload-bad-init] ... boom; plugin.plugins is still exactly [response-rewrite]; plugin.plugins_hash still contains response-rewrite; re-PUTing the identical route conf still returns 200; and /hello still returns the rewritten body.

• TEST 2 runs a failing reload followed by a good one and asserts the good one returns 200 done and the new plugin set is live — i.e. a failed reload leaves no sticky state.

• TEST 3 covers the rollback itself. reload-probe has a lower priority than reload-bad-init, so when the reload aborts its new instance has never been initialized. Asserts init=2 destroy=1 destroy_without_init=0: the old instance was destroyed once and re-init()ed by the rollback, the new one was never touched.

• TEST 4 pins the hook order. Two probes (reload-probe 411, reload-probe-2 410) record every init/destroy into the shared state; the reload adds reload-bad-init (412), which is now the highest priority of the three, so it fails before any new instance is initialized. Asserts the sequence probe:init probe-2:init probe-2:destroy probe:destroy probe:init probe-2:init — i.e. the old set is unwound LIFO and restored in init order — and covers the inited = 0 boundary, where nothing of the new set may be destroyed.

Every assertion of TEST 1 fails on current master: the endpoint returns 200, the array is [reload-bad-init] or a random remnant, the hash is empty, the route PUT returns 400 unknown plugin [response-rewrite] and /hello returns hello world. TEST 3 and TEST 4 discriminate against earlier revisions of this PR: with the whole new set destroyed on rollback TEST 3 reports destroy=2 destroy_without_init=1, and with the forward destroy restored TEST 4 reports probe:destroy probe-2:destroy. Both were verified by reverting each half of the fix separately.

Existing coverage that should stay green: t/admin/plugins-reload.t (the load plugin times: 2 / start to hot reload plugins counts are preserved by the skip-self design), t/control/plugins-reload.t, t/plugin/example.t (the plugin.load() return contract), and the by-name-timer plugins t/plugin/log-rotate.t / t/plugin/prometheus4.t.

Locally I ran t/admin/plugins-reload-transaction.t, t/control/plugins-reload.t, t/plugin/example.t and t/plugin/log-rotate.t — all green. t/admin/plugins-reload.t TEST 7 fails on my machine, but it fails identically on the base commit without this patch, so it is a local environment issue rather than a regression; t/plugin/prometheus4.t I have not run. Relying on CI for the rest.

Checklist

• I have explained the need for this PR and the problem it solves
• I have explained the changes or the new features added to this PR
• I have added tests corresponding to this change
• I have updated the documentation to reflect this change
• I have verified that this change is backward compatible (if not, please discuss on the APISIX mailing list first)