Re: Unnecessary static variable?

From: Antonin Houska <ah(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Unnecessary static variable?
Date: 2018-01-17 18:44:42
Message-ID: 22912.1516214682@localhost
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Antonin Houska <ah(at)cybertec(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-01-17 18:46:02 Re: [HACKERS] postgres_fdw bug in 9.6
Previous Message Robert Haas 2018-01-17 18:40:14 Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)