Code review comment for lp:~laurynas-biveinis/percona-server/bug1012715-5.1

Laurynas,

The assumption this patch is based on looks wrong to me, i.e.:

> + XA COMMIT. In contrast to that, the slave position is an
> + actual part of the changes made by this transaction and thus
> + must be updated in the XA PREPARE stage. */

A prepared transaction may either be committed or rolled back (see xarecover_handlerton()) depending on whether the corresponding record made it to the binary log (and if the binary log is used at all).

The correct solution would be to update slave coordinates when commiting the corresponding XA transaction on recovery, rather than moving the code storing slave coordinates to persistent storage from COMMIT to PREPARE stage. The problem is that slave coordinates are not available at the point, i.e. when committing prepared transactions on recovery.

I wonder if we can fix this by introducing another set of slave coordinates in the trx header. The we can only update those fields on PREPARE, and update the regular TRX_SYS_MYSQL_RELAY_LOG_INFO fields on COMMIT. On recovery, we could copy the "prepare" fields to "committed" ones and overwrite the relay log info file, *if* the PREPAREd transaction is being committed, e.g. in innobase_commit_by_xid(). What do you think?

Minor comments on the test case:
- do we really need the "rpl_" prefix in rpl_percona_crash_resistant_rpl.*?
- please use

SET GLOBAL debug="+d,keyword"

instead of

SET GLOBAL debug="d,keyword"

The latter breaks ./mtr --debug.