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

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-08-20T23:32:39Z
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

Attachments

On Fri, Aug 18, 2023 at 01:55:32PM +0900, Michael Paquier wrote:
> A new perl routine able to do a pg_control --includedir that scans a
> defined header with a regexp would be able to make the job for the
> constants.  If you're interested, I can code that up.

Please find attached a patch, that applies on top of the TAP test,
to implement a logic able to extract from header files the values
wanted.  This is Utils::scan_header(), for example:
my @scan_result = scan_header('server/access/xlog_internal.h',
    '#define\s+XLOG_PAGE_MAGIC\s+(\w+)');
my $XLP_PAGE_MAGIC = hex($scan_result[0]); 

This uses group capture so as it is possible to get more than one
field.  Guessing the size of the C structure is possible, but I've
found these options to be a bit awkward implementation-wise:
- Config would not help for the Postgres-specific declarations, like
TimeLineId, etc.
- I saw something that was specific to GCC..  cannot put my finger on
it now.

Another possibility would be to store the size in a different variable
in xlog_internal.h, coupled with a StaticAssertDecl() to make sure
that it matches with the real size.  While it has the same maintenance
cost as RECORD_HEADER_SIZE in the test itself, that's catching a
mismatching size at compilation-time rather than at test-time.

I am not sure that I'll be able to do more on this topic this week, at
least that's some progress.
--
Michael