On Mon, Sep 10, 2018 at 04:46:40PM +0200, Laurenz Albe wrote: > I didn't get pg_upgrade to work without the log file hacks; I suspect > that there is more than just log file locking going on, but my Windows > skills are limited. > > How shall I proceed?
I do like this patch, and we have an occasion to clean a bunch of things in pg_upgrade, so this argument is enough to me to put my hands in the dirt and check by myself, so... > I think that it is important to get pg_test_fsync to work correctly on > Windows, and if my patch does not break the buildfarm, that's what it > does. This argument argument is sound, still... [ ... suspense ... ] > I have attached a new version, the previous one was bit-rotted. I really thought that this was not ambitious enough, so I have hacked on top of your patch, so as pg_upgrade concurrent issues are removed, and I have found one barrier in pg_ctl which decides that it is smarter to redirect the log file (here pg_upgrade_server.log) using CMD. The problem is that the lock taken by the process which does the redirection does not work nicely with what pg_upgrade does in parallel. So I think that it is better to drop that part. +#ifdef WIN32 + if ((infile = fopen(path, "rt")) == NULL) +#else if ((infile = fopen(path, "r")) == NULL) +#endif This should have a comment, saying roughly that as this uses win32_fopen, text mode needs to be enforced to get proper CRLF. One spot for open() is missed in file_utils.c, please see pre_sync_fname(). The patch fails to apply for pg_verify_checksums, with a conflict easy enough to fix. At the end I would be incline to accept the patch proposed, knowing that this would fix https://postgr.es/m/16922.1520722...@sss.pgh.pa.us mentioned by Thomas upthread as get_pgpid would do things in a shared manner, putting an end at some of the random failures we've seen on the buildfarm. Laurenz, could you update your patch? I am switching that as waiting on author for now. Thanks, -- Michael
signature.asc
Description: PGP signature