Code review comment for lp:~vlad-lesin/percona-server/i_s-innodb-changed-pages

Vlad -

You are right, I misunderstood the purpose of max start and max end lsn struct, I thought that it is meant to limit the lower bound of the read interval too, although this information cannot be usefully applied yet. In this case I am not sure we need to have the max start lsn at all and we can have a single end_lsn limit variable for simpler checks, where start_lsn is handled as follows: suppose the condition is start_lsn < 100, this means we have to read all blocks with start_lsn < 100. Which is equivalent to reading all the blocks with end_lsn <= 99, or just end_lsn < 100. And we know that have read all such blocks when we read a block where start_lsn >= 100. Condition LOG_BITMAP_ITERATOR_START_LSN(i) > lsn_range.end would handle this. Does this look correct to you?

A check (whatever its final form is) in lines 627--629 should simply become a part of the while condition.

Should the "regular" ICP condition check (lines 659--660) be moved before table->field[i]->store calls?

A few too long lines in the header comment of limit_lsn_range_from_condition(): lines 476, 477, 484 and 491.

Misaligned comment starts in lines 737--747.

The log0online.h function declarations do not follow InnoDB style for their arguments. The log0online.c definitions follow them and have expanded comments, so probably should be re-copy-pasted to the header file. The header comments should be "Do something" instead of "Does something." The log_bitmap_iterator_t *i arg in all functions should be commented as "in/out" instead of "out" because it is required that i is allocated beforehand.