nic-6443 commented on issue #13087:
URL: https://github.com/apache/apisix/issues/13087#issuecomment-5237093265

   I don't think #13720 is worth landing in its current shape — the fix is a 
lot more machinery than the problem justifies.
   
   Three things pushing me there.
   
   First, nobody actually hit this. The issue is explicit that there is no 
reproduction and no error log behind it; it came out of reading `plugin.lua`. 
That doesn't make it wrong, but it should cap how much we spend on it.
   
   Second, reaching it needs a plugin that can't come up in that deployment — 
one that fails to load, or that throws in `init()` on this build/config — plus 
someone calling the reload API. A plugin like that is broken for that 
deployment either way: the next `apisix reload` or restart walks into the same 
wall, and by the numbers above it fails harder there (every request 500, 
including routes with no plugins). The plugin has to be fixed regardless, so 
all the extra machinery buys is a tidier failure on one of the three paths. I 
don't think the reload endpoint should be carrying corner-case handling for a 
state the gateway can't survive anyway.
   
   Third, and this is the main one: hot-reloading plugin code is a stopgap, not 
the supported way to ship a plugin change. If plugin code changes, the expected 
path is redeploying the gateway. Putting rollback/transaction semantics into a 
deliberately temporary escape hatch is investing in the wrong place — ~380 
lines of production code, plus lifecycle invariants (destroy-before-init, LIFO 
unwind, `package.loaded` bookkeeping) that everyone touching plugin loading 
from now on has to keep straight.
   
   If we want to do something here, the small version is enough: `pcall` around 
init/destroy, build the array and the hash together so a half-built set is 
never published, and return an error instead of `200 done`. That removes the 
genuinely bad part — a random partial plugin set hidden behind a success 
response — without introducing a transaction. Anything past that I'd leave 
until someone actually runs into it.
   


-- 
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]

Reply via email to