Hi, David! On Tue, Mar 6, 2018 at 5:04 PM, David Steele <da...@pgmasters.net> wrote:
> On 1/20/18 10:13 AM, Magnus Hagander wrote: > > > > On Tue, Nov 28, 2017 at 2:47 AM, Michael Paquier > > <michael.paqu...@gmail.com <mailto:michael.paqu...@gmail.com>> wrote: > > > > On Mon, Nov 27, 2017 at 3:28 PM, Alexander Korotkov > > <a.korot...@postgrespro.ru <mailto:a.korot...@postgrespro.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? > > This patch was mistakenly left as Needs Review during the last > commitfest but it's pretty clear that a new patch is required. > OK! No objections against marking this patch RWF. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company