Re: Loaded footgun open_datasync on Windows

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Loaded footgun open_datasync on Windows
Date: 2018-09-13 13:55:20
Message-ID: 4c53eb47273cda1543acea7a2ee1bda62b9057dc.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Use-pgwin32_open-in-frontend-code-on-Windows_V4.patch text/x-patch 3.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-09-13 13:56:02 Re: pg_dump test instability
Previous Message Alexander Kuzmenkov 2018-09-13 13:01:13 Re: Index Skip Scan