Michael Paquier wrote: > 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 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.
I got you now. So I won't try to deal with that at this point. > +#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. Done. > One spot for open() is missed in file_utils.c, please see > pre_sync_fname(). Done. > The patch fails to apply for pg_verify_checksums, with a conflict easy > enough to fix. Fixed. > Laurenz, could you update your patch? I am switching that as waiting on > author for now. Attached is the new, improved version of the patch. Thanks again! Yours, Laurenz Albe
From 8cd3d1a75fc3b18e6928b9b26841f1947d1f72ba Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Thu, 13 Sep 2018 14:24:59 +0200 Subject: [PATCH] Use pgwin32_open in frontend code on Windows This is particularly important for pg_test_fsync which does not handle O_DSYNC correctly otherwise, leading to false claims that disks are unsafe. With pgwin32_open, files won't get locked when opened. This could be used to get rid of some of the workarounds for Windows' file locking, but that is left for later because it proved not to be as trivial as hoped. Discussion: https://www.postgresql.org/message-id/1527846213.2475.31.camel%40cybertec.at Laurenz Albe, reviewed by Michael Paquier and Kuntal Ghosh --- src/bin/initdb/initdb.c | 8 ++++++++ src/bin/pg_basebackup/pg_receivewal.c | 2 +- src/bin/pg_verify_checksums/pg_verify_checksums.c | 2 +- src/common/file_utils.c | 4 ++-- src/include/port.h | 3 --- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 32746c7703..bf234c0763 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -490,7 +490,15 @@ readfile(const char *path) char *buffer; int c; +#ifdef WIN32 + /* + * On Windows, we have to open the file in text mode + * so that "carriage return" characters are stripped. + */ + if ((infile = fopen(path, "rt")) == NULL) +#else if ((infile = fopen(path, "r")) == NULL) +#endif { fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), progname, path, strerror(errno)); diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index 8be8d48a8a..912aed8d7c 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -288,7 +288,7 @@ FindStreamingStart(uint32 *tli) snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name); - fd = open(fullpath, O_RDONLY | PG_BINARY); + fd = open(fullpath, O_RDONLY | PG_BINARY, 0); if (fd < 0) { fprintf(stderr, _("%s: could not open compressed file \"%s\": %s\n"), diff --git a/src/bin/pg_verify_checksums/pg_verify_checksums.c b/src/bin/pg_verify_checksums/pg_verify_checksums.c index d46bac2cd6..507f83ca4c 100644 --- a/src/bin/pg_verify_checksums/pg_verify_checksums.c +++ b/src/bin/pg_verify_checksums/pg_verify_checksums.c @@ -80,7 +80,7 @@ scan_file(const char *fn, BlockNumber segmentno) int f; BlockNumber blockno; - f = open(fn, O_RDONLY | PG_BINARY); + f = open(fn, O_RDONLY | PG_BINARY, 0); if (f < 0) { fprintf(stderr, _("%s: could not open file \"%s\": %s\n"), diff --git a/src/common/file_utils.c b/src/common/file_utils.c index 48876061c3..d952bc8c88 100644 --- a/src/common/file_utils.c +++ b/src/common/file_utils.c @@ -222,7 +222,7 @@ pre_sync_fname(const char *fname, bool isdir, const char *progname) { int fd; - fd = open(fname, O_RDONLY | PG_BINARY); + fd = open(fname, O_RDONLY | PG_BINARY, 0); if (fd < 0) { @@ -283,7 +283,7 @@ fsync_fname(const char *fname, bool isdir, const char *progname) * unsupported operations, e.g. opening a directory under Windows), and * logging others. */ - fd = open(fname, flags); + fd = open(fname, flags, 0); if (fd < 0) { if (errno == EACCES || (isdir && errno == EISDIR)) diff --git a/src/include/port.h b/src/include/port.h index d92756111f..2522ebc35f 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -249,11 +249,8 @@ extern bool rmtree(const char *path, bool rmtopdir); #define O_DIRECT 0x80000000 extern int pgwin32_open(const char *, int,...); extern FILE *pgwin32_fopen(const char *, const char *); - -#ifndef FRONTEND #define open(a,b,c) pgwin32_open(a,b,c) #define fopen(a,b) pgwin32_fopen(a,b) -#endif /* * Mingw-w64 headers #define popen and pclose to _popen and _pclose. We want -- 2.17.0
