Andrei Elkin <andrei.elkin@mariadb.com> writes:

...

I think it's better to define a proper API for this.

It feels an overkill in the current case.

"Define a proper API" means that you think through how this should work for different storage engines and TC_LOG implementations. And then you document those thoughts. I don't understand how doing this can be overkill, you are re-defining how durability of external XA transactions are coordinated between the server layer and the storage engines.

...

And vice versa, for every xid X_COMPLETE found, roll it back if X is found prepared in InnoDB,

Actually

...

else commit it. safer must be to commit both X_COMPLETE and X. Generally even if X_COMPLETE and X commits in this order, X can race to be durable before crash.

Right, and "commit X" would mean "XA ROLLBACK X" if the "complete" operation is a rollback. I didn't follow why rolling back the X_COMPLETE would be a problem, but it's not important for now; I can see how committing should work as well.

...

One reason I'm asking is I'm wondering how this will affect the scheduling of XA in parallel replication? Like, in case of duplicate XID, the X_PREPARE or X_COMPLETE could conflict between two transactions, so maybe the wakeup_subsequent_commits can only be done after the XA COMMIT of the X_PREPARE and X_COMMIT? Which I think would require changes to the current patch.

Hmm, right now in the branch (see Sliding_window etc) for the duplicate XID case, like P_1,C_2,P_3,C_4 P - XA-PREPARE, C - ... commit) P_3 can start only when C_2 wakes it up.

Yes. So my point is: In this case, the C_2 cannot do wakeup_subsequent_commits inside the group commit along with run_commit_ordered(), that would be too early. C_2 must first complete the extra internal XA transaction on the mysql.gtid_slave_pos table before it can wakeup P_3. Or is this not a problem?

My bad I left out a mention of the plan, to treat - call it XAC_0 -> XAP_1 dependency - by the optimistic retry. That's in order to minimize the

Aha. So if we optimistically apply the XA PREPARE, and we get an error ER_XAER_DUPID that the XID already exists as prepared, then we would do a retry like other temporary errors, is that what you mean? Is this implemented in the current patch? Maybe it is automatically handled by convert_kill_to_deadlock_error() ? If this XAC_0 -> XAP_1 conflict can be handled by normal optimistic rollback/retry, then I agree there is no need to handle the dependency explicitly in the sliding window. That was not clear to me from the code.

...

This I don't understand at all. How can an XA PREPARE/COMMIT/ROLLBACK be marked as "ddl" ?

That's a binlogging whim. E.g a temp table "DDL" involved into transaction. I was not sure I could quickly eliminate the flagging, so decided to live with it.

Aha, right, CREATE TEMPORARY TABLE can be part of a transaction and thus DDL (in statement-based binlogging). I missed that.

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.

If binlog is globally enabled (--log-bin option), then tc_log is an object of class MYSQL_BIN_LOG. Else it is class TC_LOG_MMAP (or TC_LOG_DUMMY). So a virtual function can be used instead of a conditional on opt_bin_log. But binlogging can be disabled on a transaction-by-transaction basis by setting SQL_LOG_BIN, is that the case here? Then yes, it seems an explicit condition will be needed. - Kristian.