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

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Sergei Kornilov <sk@zsrv.org>
Cc: Michael Paquier <michael@paquier.xyz>, 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-09-04T03:20:31Z
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

Thanks for looking/testing, Sergei.  Thanks for the changes, Michael,
these all look good.  I've squashed them and added you as co-author.

A couple more small comment/text changes:

1.  In the place where we fail to allocate memory for an oversized
record, I copied the comment about treating that as a "bogus data"
condition.  I suspect that we will soon be converting that to a FATAL
error[1], and that'll need to be done in both places.

2.  In this version of the commit message I said we'd only back-patch
to 15 for now.  After sleeping on this for a week, I realised that the
reason I keep vacillating on that point is that I am not sure what
your plan is for the malloc-failure-means-end-of-wal policy ([1],
ancient code from 0ffe11abd3a).  If we're going to fix that in master
only but let sleeping dogs lie in the back-branches, then it becomes
less important to go back further than 15 with THIS patch.  But if you
want to be able to distinguish garbage from out-of-memory, and thereby
end-of-wal from a FATAL please-insert-more-RAM condition, I think
you'd really need this industrial strength validation in all affected
branches, and I'd have more work to do, right?  The weak validation we
are fixing here is the *real* underlying problem going back many
years, right?

I also wondered about strengthening the validation of various things
like redo begin/end LSNs etc in these tests.  But we can always
continue to improve all this later...

Here also is a version for 15 (and a CI run[2]), since we tweaked many
of the error messages between 15 and 16.

[1] https://www.postgresql.org/message-id/flat/ZMh/WV%2BCuknqePQQ%40paquier.xyz
[2] https://cirrus-ci.com/task/4533280897236992 (failed on some
unrelated pgbench test, reported in another thread)