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

> Some feature design and code questions. Maybe not all of them need to
> be resolved right now.
>
> - We need to know somehow if the current run is complete, i.e. if we
> query some LSN interval, the last block we read should have the last
> block bit set, if we silently output partial data, the user will not
> know if he should re-query the last part of LSN interval or not. I
> am not sure what would be the best way. Another column for last
> block bit flag? That would be quite clumsy for users to use.

Maybe it's better to wait until log tracking thread completely write all blocks for certain lsn interval?

About holes. We could to have another I_S for holes and bad blocks recognition. Another option you described below:

> - Likewise I am not sure what should we do about corrupted blocks in
> the middle of requested LSN intervals (and maybe missing LSN
> interval data too). Another boolean column "this block is continuous
> from the previous block"?

It's easy to recognize bad blocks just ckecksum checking. And we could read lsn interval completely before output which allows us to recognize not completely written lsn intervals checking "last block" flag at the end of interval. We could use linked list of changed pages inside of iterator instead of just one page to hold lsn range.

> - In any case I guess we should diagnose bad block checksums we
> encounter to the error log at least. That's easy, but it raises the
> issue of what would happen if the I_S query would attempt to read the
> last bitmap block while it's being written. Currently such block
> would be ignored correctly, but in this case we would get false
> errors. I think we should introduce a log online mutex for the last
> written block protection. Acquire it in the log writer around the
> block write + sync. Acquire it (and immediatelly release) in the
> log iterator right before the read when (offset == file size - 1
> block).

Yes, mutex would help us to avoid reading not completely written blocks. But what about partially written lsn intervals? Maybe that mutex should protect the whole interval writing? Not only one block?

> - What is the purpose of LOG_BITMAP_ITERATOR_* macros? Except for
> LOG_BITMAP_ITERATOR_PAGE_NUM

The purpose is to hide implementation of iterator. We can change the content of iterator and some work logic not changing the code which uses the iterator.