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

On 07/04/2012 05:50 PM, Laurynas Biveinis 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?
>
> Good idea, I will work on this if the points below are agreed upon. I will probably use the explicit XA syntax.
>

There are also existing injection sites that you may want to use, e.g.
crash_commit_after_prepare will crash the server after preparing a
transaction, but before writing a xid event to binlog, so that would
theoretically lead to a rollback on recovery.

>> 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
>
> My understanding is that binlog position record in InnoDB means "InnoDB transactions are committed (not prepared) up to this binlog position." Thus it cannot possibly go back and in contrast to the slave info log, it is not part of the transaction itself, but rather InnoDB/binlog metadata of sorts. I briefly experimented with moving this to PREPARE too and broke crash recovery even worse. I can research to provide more info if you want me to.
>

OK, I see what you meant to say, no need for details. Thanks for
clarifications.