Re: [PATCH] Atomic pgrename on Windows

On Tue, Nov 28, 2017 at 2:47 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Nov 27, 2017 at 3:28 PM, Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > Attached patch atomic-pgrename-windows-1.patch fixes this problem. It
> > appears to be possible to atomically replace file on Windows –
> ReplaceFile()
> > does that. ReplaceFiles() requires target file to exist, this is why we
> > still need to call MoveFileEx() when it doesn't exist.
>
> Do you think that it could be safer to unlink the target file first
> with pgunlink()? This way you make sure that the target file is
> removed and not locked. This change makes me worrying about the
> introduction of more race conditions.

Unlinking it first seems dangerous, as pointed out by Andres.

What about first trying ReplaceFile() and then if it fails with "target
doesn't exist", then call MoveFileEx().

Or the other way around -- try MoveFileEx() first since that seems to work
most of the time today (if it mostly broke we'd be in trouble already), and
if it fails with a sharing violation, try ReplaceFile()? And perhaps end up
doing it something similar to what we do with shared memory which is just
to loop over it and try each a couple of time, before giving up and
failing?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>