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

> > - 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?

Such wait could be very long on a busy server that starves the log following thread.

> About holes. We could to have another I_S for holes and bad blocks
> recognition.

Can you describe in more detail how you imagine this another I_S table?

> 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.

But the issue is not the detection of bad or missing data, but rather what to do when this situation happens.

> > - 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?

It probably depends on what we decide to do for the above questions.

> > - 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.

Ah, I see, thanks. I assumed this was not the case, because the struct itself was in a header file.