Hi Andrey, Markos,
Thank you for reviewing this. Here is a new patch fixing the bug that
Andrey found. I need to get back to you regarding the benchmarks.
Best wishes,
Mats Kindahl
On 9/14/26 20:44, Andrey Borodin wrote:
> Hi Mats, Markos,
>
> Mats, in the original gp3 setup, did you also measure fewer device-level
> reads and shorter recovery time with the normal readahead settings?
>
> I'd also be interested in replay performance with WAL and relation pages
> already cached. Fewer syscalls could help even when the kernel already
> combines disk reads, but does that saving outweigh the extra per-page
> memcpy in this implementation?
>
> Looking at v1, I think there is a cache-validity issue with streaming:
>
>> + if (readSource == XLOG_FROM_STREAM)
>> + wanted = Min(wanted, readLen);
>> ...
>> + wanted = Max(wanted, XLOG_BLCKSZ);
> readLen is the valid prefix of the requested page, not the amount of
> available WAL from that page onwards. It is at most XLOG_BLCKSZ, so
> these bounds also leave streaming reads at one page regardless of
> io_combine_limit.
>
> More importantly, when only part of the page has arrived, pread() still
> reads the whole page and readAheadLen records the full result. Once
> more WAL arrives, XLogPageRead() can be called again for the same page
> with a larger reqLen. The new cache then hits and returns the old copy,
> including bytes that were not valid when it was filled.
>
> Should the cache track how many bytes were valid at the time of the
> read, and refill when reqLen exceeds that prefix? The available WAL
> distance from targetPagePtr to flushedUpto could separately bound the
> combined read size.
>
> Thanks!
>
>
> Best regards, Andrey Borodin.