Re: Do away with zero-padding assumption before WALRead()

On Fri, Feb 16, 2024 at 7:10 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Good catch! The comment seems wrong also to me. The subsequent bytes
> can be written simultaneously, and it's very normal that there are
> unflushed bytes are in OS's page buffer. The objective of the comment
> seems to be to declare that there's no need to clear out the remaining
> bytes, here. I agree that it's not a problem for now. However, I think
> we need two fixes here.
>
> 1. It's useless to copy the whole page regardless of the 'count'. It's
> enough to copy only up to the 'count'. The patch looks good in this
> regard.

Yes, it's not needed to copy the whole page. Per my understanding
about page transfers between disk and OS page cache - upon OS page
cache miss, the whole page (of disk block size) gets fetched from disk
even if we just read 'count' bytes (< disk block size).

> 2. Maybe we need a comment that states the page_read callback
> functions leave garbage bytes beyond the returned count, due to
> partial copying without clearing the unused portion.

Isn't the comment around page_read callback at
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/access/xlogreader.h;h=2e9e5f43eb2de1ca9ba81afe76d21357065c61aa;hb=d57b7cc3338e9d9aa1d7c5da1b25a17c5a72dcce#l78
enough?

"The callback shall return the number of bytes read (never more than
XLOG_BLCKSZ), or -1 on failure."

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com