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

On 09.08.12 6:30, Laurynas Biveinis wrote:
>
> Isn't ha_innodb.cc C++? But I will fix them in any case.
>

Right, it's C++, sorry.

>> - in general, I would suggest to fix all of the above by leaving the
>> (incorrectly formatted) code alone and not moving it into a
>> separate function. That would save me about half an hour on reading
>> and commenting on changes that are not really changes but rather
>> moving the code around, and allow me to focus on what's really been
>> changed.
>> - same goes for fname -> info_fname renaming
>
> There are two places where I extracted new functions from existing code: in ha_innodb.cc and in trx0sys.c. Which one of them, or both, are you referring to? The trx0sys.c one I can revert, but IMHO it is easy to review too. Re. ha_innodb.cc changes, I have extracted innobase_do_overwrite_relay_log_info() because I needed to call it from another function as well and I don't see a good alternative to making a new function: if I copy pasted the code, I'd still need to adjust it heavily due to different local var context and the end result would be very close to innobase_do_overwrite_relay_log_info() anyway then. And I did the fname/pos variable rename & split, because these variables being repurposed five times is way beyond my pain threshold and extracted from innobase_setup() they stop working anyway.
>

I was referring to innobase_do_overwrite_relay_log_info().

The second call of the function looked like a part of the comment to me
when I grepped:

+ /* On rollback of a prepared transaction revert the
+ current slave positions to the ones recorded by the
+ last COMMITTed transaction. This has an effect of
+ undoing the position change caused by the transaction
+ being rolled back. Assumes single-threaded slave SQL
+ thread. If the server has non-master write traffic
+ with XA rollbacks, this will cause additional spurious
+ slave info log overwrites, which should be harmless. */
+ trx_sys_print_committed_mysql_master_log_pos();
+ innobase_do_overwrite_relay_log_info();

In the InnoDB code multi-line comments are usually separated from code
with blank lines. That is easier to read.