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