On Mon, Jun 15, 2020 at 11:49:33PM -0300, Ranier Vilela wrote: > I can confirm that the problem is in pgrename (dirmod.c), > something is not OK, with MoveFileEx, even with the > (MOVEFILE_REPLACE_EXISTING) flag. > > Replacing MoveFileEx, with > unlink (to); > rename (from, to); > > #if defined (WIN32) &&! defined (__ CYGWIN__) > unlink(to); > while (rename (from, to)! = 0) > #else > while (rename (from, to) <0) > #endif > > The log messages have disappeared. > > I suspect that if the target (to) file exists, MoveFileEx, it is failing to > rename, even with the flag enabled. > > Windows have the native rename function ( > https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/rename-wrename?view=vs-2019 > ) > However, it fails if the target (to) file exists. > > Question, is it acceptable delete the target file, if it exists, to > guarantee the rename?
I don't think so - the idea behind writing $f.tmp and renaming it to $f is that it's *atomic*. I found an earlier thread addressing the same problem - we probably both should've found this earlier. https://commitfest.postgresql.org/27/2230/ https://www.postgresql.org/message-id/flat/CAPpHfds9trA6ipezK3BsuuOSQwEmESiqj8pkOxACFJpoLpcoNw%40mail.gmail.com#9b04576b717175e9dbf03cc991977d3f That thread goes back to 2017, so I don't think this is a new problem in v13. I'm not sure why you wouldn't also see the same behavior in v12. There's a couple patches in that thread, and the latest patch was rejected. Maybe you'd want to test them out and provide feedback. BTW, the first patch does: ! if (filePresent) ! { ! if (ReplaceFile(to, from, NULL, REPLACEFILE_IGNORE_MERGE_ERRORS, 0, 0)) ! break; ! } ! else ! { ! if (MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING)) ! break; ! } Since it's racy to first check if the file exists, I would've thought we should instead do: ret = ReplaceFile() if ret == OK: break else if ret == FileDoesntExist: if MoveFileEx(): break Or, should we just try to create a file first, to allow ReplaceFile() to always work ? -- Justin
