Horton

Re: Back-patch of: avoid multiple hard links to same WAL file after a crash

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Nathan Bossart <nathandbossart@gmail.com>, Andres Freund <andres@anarazel.de>, Robert Pang <robertpang@google.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, robertmhaas@gmail.com, pgsql-hackers@postgresql.org
Date: 2025-04-06T02:09:58Z
Lists: pgsql-hackers 
On Sun, Apr 06, 2025 at 07:42:02AM +0900, Michael Paquier wrote:
> On Sat, Apr 05, 2025 at 12:13:39PM -0700, Noah Misch wrote:
> > Since the 2025-02 releases made non-toy-size archive recoveries fail easily,
> > that's not enough.  If the proposed 3-second test is the wrong thing, what
> > instead?
> 
> I don't have a good idea about that in ~16, TBH, but I am sure to not
> be a fan of the low reproducibility rate of this test as proposed.
> It's not perfect, but as the design to fix the original race condition
> has been introduced in v15, why not begin with a test in 17~ using
> some injection points?

Two reasons:

a) The fix ended calls to the whole range of relevant code.  Hence, the
   injection point placement that would have been relevant before the fix
   isn't reached.  In other words, there's no right place for the injection
   point.  (The place for the injection point would be in durable_rename(), in
   the checkpointer.  After the fix, the checkpointer just doesn't call
   durable_rename().)

b) Stochastic tests catch defects beyond the specific one the test author
   targeted.  An injection point test is less likely to do that.  (That said,
   with reason (a), there's no known injection point test design to compete
   with the stochastic design.)

Commits

  1. Test restartpoints in archive recovery.

  2. Reset InstallXLogFileSegmentActive after walreceiver self-initiated exit.

  3. Skip WAL recycling and preallocation during archive recovery.

  4. Don't ERROR on PreallocXlogFiles() race condition.

  5. Revert "Add HINT for restartpoint race with KeepFileRestoredFromArchive()."

  6. Remove XLogFileInit() ability to unlink a pre-existing file.

  7. In XLogFileInit(), fix *use_existent postcondition to suit callers.

  8. Remove XLogFileInit() ability to skip ControlFileLock.

  9. Replace durable_rename_excl() by durable_rename(), take two

  10. Add HINT for restartpoint race with KeepFileRestoredFromArchive().

  11. Remove durable_rename_excl()

  12. Replace existing durable_rename_excl() calls with durable_rename()