In the 3rd bunch of notes here is a last non-recovery related one
I need to digest that in the morning. Generally I don't see a problem to conduct hton release flexibly.
that I was going to comment on.
-static my_bool xacommit_handlerton(THD *unused1, plugin_ref plugin, - void *arg) +my_bool xacommit_handlerton(THD *unused1, plugin_ref plugin, void *arg) { handlerton *hton= plugin_hton(plugin); - if (hton->recover) + if (hton->recover && hton != binlog_hton) { hton->commit_by_xid(hton, ((struct xahton_st *)arg)->xid);
-int ha_commit_or_rollback_by_xid(XID *xid, bool commit) +int ha_commit_or_rollback_by_xid(XID *xid, bool commit, THD *thd) {
+ if (!skip_binlog) + { + // when binlog is ON start from its transaction branch if (commit) binlog_commit_by_xid(binlog_hton, xid); else binlog_rollback_by_xid(binlog_hton, xid);
I think the purpose if this is to ensure that commit/rollback of XA calls into the binlog code first (to binlog before doing it in the engines).
But as I think Serg also remarked already, this should not be needed. The binlog is the (internal) transaction coordinator when it is enabled, we have a dedicated interface for that, class TC_LOG. This is used elsewhere in handler.cc to avoid special cases for whether the binlog is enabled or not.
So it looks like there needs to be extra virtual functions in TC_LOG for the XA commit/rollback, and these can then be called on the tc_log object and do what is required.
I need to process that tomorrow.
I agree this can be wrapped up into a new TC_LOG::commit_or_rollback_by_xid() which would then take care of invoking a common function with binlog_commit() function first, which would further direct into TC_LOG::run_commit_ordered(). That looks a more modular approach, indeed. But I could not get the meaning of
... handler.cc to avoid special cases for whether the binlog is enabled The burden of checking whether the binlog is enabled, e.g for taking a TC_LOG->log_and_order() path is on the programmer.
Speaking about that I would fancy about class hierarchy that would include a dummy TC_LOG of the disabled binlog that would dynamically resolve the right class' method...