Horton

Re: BUG #17928: Standby fails to decode WAL on termination of primary

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

View thread
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: noah@leadboat.com
Cc: thomas.munro@gmail.com, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-07-20T06:01:42Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Correct assertion and comments about XLogRecordMaxSize.

  2. Fix edge-case for xl_tot_len broken by bae868ca.

  3. Don't use Perl pack('Q') in 039_end_of_wal.pl.

  4. Don't trust unvalidated xl_tot_len.

  5. Make recovery report error message when invalid page header is found.

  6. Add more protections in WAL record APIs against overflows

At Sun, 16 Jul 2023 17:49:05 -0700, Noah Misch <noah@leadboat.com> wrote in 
> On Tue, Jul 11, 2023 at 03:54:14PM +0900, Kyotaro Horiguchi wrote:
> > I like the patch for its code clean up, but I'm afraid that it removes
> > the existing record length check when reading continued
> > pages.
> 
> The removed check would be repetitive in all cases known to me, but I wouldn't
> mind keeping the check instead.

Ah. I got it. No need to do that again.

> > I believe reading the whole header is the most sensible approach as it
> > can prevent unnecessary memory requests. Another potential solution
> > (or hack) for this specific case is to let XLogWalRcvFlush write a
> > finalizing ((uint32)0) when dying is true. This stabilizes the
> > behavior to "invalid record length.. got 0" when running the TAP test.
...
> That's inexpensive in cycles, and it should make the failures in question
> quite rare.  If we do that instead of pre-zeroing, I think we'd still want to
> be ready for invalid lengths, for the case of unclean standby shutdown.

Agreed.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center