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

Revision history for this message
Alexey Kopytov (akopytov) wrote :

On 07/03/2012 08:35 PM, Laurynas Biveinis wrote:
=> Namely, why is the assumption "slave position update is a part of
actual transaction changes" wrong in the light of this? If a prepared
transaction is rolled back, the old slave position is restored from an
undo seg. If it is 2pc-committed, the new slave position becomes permanent.
>

Right, it didn't occur to me that updates of the trx header fields are
done as a part of the current transaction. If they are rolled back when
a prepared transaction is rolled back, then the fix is correct. Can we
have it covered by the test case?

Though it's not clear then what the difference with the binlog position
update is, i.e. what following means:

> + /* Update the replication position info inside InnoDB. This is
> + different from the binlog position update that happens during
> + XA COMMIT. In contrast to that, the slave position is an

> Maybe you view this fix as incomplete or working by chance because of the following (which is the actual action sequence that happens in this bug): on crash recovery the relay status log overwrite will happen before the XA rollback, thus the slave position will point to as if the transaction was fully committed. Then the transaction will be rolled back, (which would require overwriting relay status log with the old position) and replayed from the binlog, (which would require overwriting the relay status log position again, but binlog does not have the required info for that). This results in correct positions, although with a shortcut taken. The assumption here that all replicated InnoDB XA prepared transactions will be eventually committed. It is not perfect but IMHO "slave position is a part of transaction itself" is a step to the right direction.
>

I don't understand this. When and how a replay from binlog of a rolled
back XA transaction occurs? If I'm not mistaken, a roll back happens
when the corresponding event is _not_ in the binary log (or binlog not
used at all).

« Back to merge proposal