Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Antonin Houska <ah@cybertec.at> writes:
> but your patch also does this:
Yes, that should have been a separate diff.
> > *************** retry:
> > *** 11648,11654 ****
> > }
> >
> > pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
> > ! if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
> > {
> > char fname[MAXFNAMELEN];
> >
> > --- 11644,11650 ----
> > }
> >
> > pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
> > ! if (read(readFile, readBuf, readLen) != readLen)
> > {
> > char fname[MAXFNAMELEN];
>
> and that I'm less sure is correct.
>
> At one time, I think, readLen told how much data in readBuf was
> actually valid. It seems not to be used for that anymore, but
> I don't much like the idea that readBuf is only partially filled
> but there is *no* persistent state indicating how much is valid.
> The existing coding guarantees that the answer is "XLOG_BLCKSZ",
> so that's fine, but this change would remove the guarantee.
XLogPageRead() is a callback of the XLOG reader and that passes reqLen telling
how much data of the page is actually needed in readBuf at the moment. And the
function checks that readLen is high enough:
Assert(reqLen <= readLen);
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com