public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: avoid multiple hard links to same WAL file after a crash
Date: Thu, 5 May 2022 20:10:02 +0900
Message-ID: <YnOwioN/[email protected]> (raw)
In-Reply-To: <20220502230613.GA3398932@nathanxps13>
References: <CA+TgmobGi7b-5fjAWgEO0ebn516n0OyOi-j7+Hyz_j9sAG17og@mail.gmail.com>
	<[email protected]>
	<20220411165257.GB1915258@nathanxps13>
	<[email protected]>
	<20220412162742.GB2062175@nathanxps13>
	<[email protected]>
	<[email protected]>
	<20220502173907.GB3351307@nathanxps13>
	<20220502230613.GA3398932@nathanxps13>

On Mon, May 02, 2022 at 04:06:13PM -0700, Nathan Bossart wrote:
> Here is a new patch set.  For now, I've only removed the file existence
> check in writeTimeLineHistoryFile().  I don't know if I'm totally convinced
> that there isn't a problem here (e.g., due to concurrent .ready file
> creation), but since some platforms have been using rename() for some time,
> I don't know how worried we should be.

That's only about Windows these days, meaning that there is much less
coverage in this code path.

> I thought about adding some kind of
> locking between the WAL receiver and startup processes, but that seems
> excessive.

Agreed.

> Alternatively, we could just fix xlog.c as proposed earlier
> [0].  AFAICT that is the only caller that can experience problems due to
> the multiple-hard-link issue.  All other callers are simply renaming a
> temporary file into place, and the temporary file can be discarded if left
> behind after a crash.

I'd agree with removing all the callers at the end.  pgrename() is
quite robust on Windows, but I'd keep the two checks in
writeTimeLineHistory(), as the logic around findNewestTimeLine() would
consider a past TLI history file as in-use even if we have a crash
just after the file got created in the same path by the same standby,
and the WAL segment init part.  Your patch does that.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../YnOwioN%[email protected]/2-signature.asc)
  download

view thread (21+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: avoid multiple hard links to same WAL file after a crash
  In-Reply-To: <YnOwioN/[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox