Hi Jakub,
Thanks for reviewing it. I'm resending the chain of patches here, with 0005 already merged + your suggestions applied.
Here is a summary of what I've fixed:
> I've forgotten to support incremental mode (which means that I ended up issuing posix_fadvise SEQUENTIAL even in incremental mode
1. Fixed. Only issue posix_fadvise SEQUENTIAL for full backups and only issue posix_fadvise WILLNEED for incremental backups.
> there's bug that when m_io_c = 0 you still issue posix_fadvise()s from 2nd callsite (no way to turn it off) and also with e.g. m_io_c = 1 we seem to issue an additional one.
2. Fixed this by only calling prefetch_next_incremental_run when m_io_c > 1. Also moved the call to happen after read_file_data_into_buffer instead of before. Both changes together make the prefetching logic a little more honest regarding I/O queue depth, i.e:
- When m_io_c = 0 or m_io_c = 1: no prefetching at all, queue depth is constantly1 (the synchronous block read).
- When m_io_c > 1 (e.g. 2): prefetch 2 initially > read 1 + prefetch 1 > read 1 + prefetch 1... on average, there will always be 2 I/O simultaneously (assuming fadvise calls are honored).
> I'm not sure f that merging of adjacent blocks shouldn't be also capped up a io_combine_limit GUC
3. Agree that capping is good there. Though I think maintenance_io_concurrency can be reused for this purpose instead? I think a new GUC for this might be too specific. I currently have it implemented using m_io_c. Let me know your thoughts on this if you think otherwise.
> I would rename pf_index to just prefetch_idx, so it would easier to read.
4. Agreed. Fixed.
Thanks,
Gustavo Oliveira
