Re: avoid multiple hard links to same WAL file after a crash

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Greg Stark <stark@mit.edu>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-04-27T18:42:04Z
Lists: pgsql-hackers

Attachments

On Wed, Apr 27, 2022 at 04:09:20PM +0900, Michael Paquier wrote:
> I am not sure that have any need to backpatch this change based on the
> unlikeliness of the problem, TBH.  One thing that is itching me a bit,
> like Robert upthread, is that we don't check anymore that the newfile
> does not exist in the code paths because we never expect one.  It is
> possible to use stat() for that.  But access() within a simple
> assertion would be simpler?  Say something like:
> Assert(access(path, F_OK) != 0 && errno == ENOENT);
> 
> The case for basic_archive is limited as the comment of the patch
> states, but that would be helpful for the two calls in timeline.c and
> the one in xlog.c in the long-term.  And this has no need to be part
> of fd.c, this can be added before the durable_rename() calls.  What do
> you think?

Here is a new patch set with these assertions added.  I think at least the
xlog.c change ought to be back-patched.  The problem may be unlikely, but
AFAICT the possible consequences include WAL corruption.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

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()