On Fri, Feb 16, 2024 at 7:10 AM Kyotaro Horiguchi <horikyota....@gmail.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