Hi Sergei, Thanks for the comments. I've addressed them below - see my latest comment in the ticket for the updated commit. On Fri 2024-01-05 17:57:40 +0100, Sergei Golubchik wrote:
[... 27 lines elided]
+ /* Plugin initialisation done here should defer any ALTER TABLE + queries to after the ddl recovery is done, in the + signal_ddl_recovery_done() callback called by + ha_signal_ddl_recovery_done(). As such, between the plugin_init() + call and the ha_signal_ddl_recovery_done() call below only things + related to prepare for recovery should be done and nothing else, and + definitely not anything assuming that all plugins have been + initialised. */
Better put this comment in plugin.h near the (*init)(void*) declaration. This is part of the (*init)() callback documentation it should be there, not in some random .cc file that plugin authors shouldn't be required to look at.
Done. This code documentation consists of a part for people working on init_server_components() and (as you mentioned here) a part for people working on plugins. So I split it into two and placed the second part in the (*init)(void*) documentation.
[... 19 lines elided]
may be you don't need a new argument for plugin_foreach_with_mask, and can put this logic into the `func` callback? That is into
static my_bool signal_ddl_recovery_done(THD *, plugin_ref plugin, void *) { handlerton *hton= plugin_hton(plugin); if (hton->signal_ddl_recovery_done) if ((hton->signal_ddl_recovery_done)(hton)) plugin_ref_to_int(plugin)->state= PLUGIN_IS_DELETED; return 0; }
or something like that.
Done.
[... 21 lines elided]
I don't see much of a point, why not to run *all* queries from spider_ddl_recovery_done() ?
Done - basically preserving what you did in 0930eb86cb00edb2a.
[... 30 lines elided]
Don't do that, instead, please, take the "ddl_recovery_done" change from the commit 0930eb86cb00edb2a - it makes the storage engine API simpler, the engine doesn't need to implement two code paths and try to detect whether it's loaded at run-time or at startup.
Done.
[... 10 lines elided]
Best, Yuchen