Hi,
On 2021-03-16 16:20:37 +0900, Michael Paquier wrote:
> Fujii-san has mentioned that on twitter, but one area that has changed
> during the v13 cycle is aaa3aed, where the code recycling segments has
> been switched from a pgrename() (with a retry loop) to a
> CreateHardLinkA()+pgunlink() (with a retry loop for the second). One
> theory that I got in mind here is the case where we create the hard
> link, but fail to finish do the pgunlink() on the xlogtemp.N file,
> though after some testing it did not seem to have any impact.
A related question: What on earth is the point of using the unlink
approach on any operating system. We use the durable_rename_excl() (and
its predecessor, durable_link_or_rename(), and in turn its open coded
predecessors) for things like recycling WAL files at check points.
Now imagine that durable_rename_excl() fails to unlink the old
file. We'll still have the old file, but there's a second link to a new
WAL file, which will be used. No error will be thrown, because we don't
check unlink()'s return code (but if we did, we'd still have similar
issues).
And then imagine that that happens again, during the next checkpoint,
because the permission or whatever issue is not yet resolved. We now
will have the same physical file in two location in the future WAL
stream.
Welcome impossible to debug issues.
And all of this with the sole justification of "paranoidly trying to
avoid overwriting an existing file (there shouldn't be one).". A few
lines after we either unlinked the target filename, or used stat() to
find an unused filename.
Isn't the whole idea of durable_rename_excl() bad? There's not the same
danger of using it when we start from a temp filename, e.g. during
creation of new segments, or timeline files or whatnot. But I still
don't see what the whole thing is supposed to protect us against
realistically.
Greetings,
Andres Freund
