Re: pgsql: Allow concurrent-safe open() and fopen() in frontend code for Wi

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Allow concurrent-safe open() and fopen() in frontend code for Wi
Date: 2018-09-19 02:32:11
Message-ID: 20180919023211.GC1650@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Tue, Sep 18, 2018 at 06:04:58PM +0200, Laurenz Albe wrote:
> That wouldn't influence pipes, which was what Michael said was a
> problem for pg_dump.

Yeah, the authentication blows up badly on that.. You can see all the
tests using user and database names with all ASCII range turning red.

> I currently have no Windows system close, so I cannot test...

I have spent a large portion of my morning trying to test all the
solutions proposed, and a winner shows up. Attached are three patches
which present all the solutions mentioned, attached here for visibility:
- win32-open-michael.patch, or the solution I was working on. This has
led me actually nowhere. Even trying to enforce _fmode to happen only
on the frontend has caused breakages of pg_dump for authentication.
Trying to be smarter than what other binaries do is nice and consistent
with the Windows experience I think, still it looks that this can lead
to breakages when a utility uses setmode() for some of the output
files. I noticed that pgwin32_open missed to enforce the mode used when
calling _fdmode, still that solves nothing.
- win32-open-tom.patch, which switches pgwin32_fopen() to use text mode
all the time if binary is not specified. While this looked promising,
I have noticed that this has been causing the same set of errors as what
my previous patch has been doing in pg_dump TAP tests. Anyway, a
solution needs also to happen for pgwin32_open() as we want direct calls
to it to get the right call.
- win32-open-laurenz.patch, which enforces to text mode only if binary
mode is not defined, which maps strictly to what pre-11 is doing when
calling the system _open or _fopen. And surprisingly, this is proving
to pass all the tests I ran: bincheck (including pgbench and pg_dump),
upgradecheck, recoverycheck, check, etc. initdb --pwfile is not
complaining to me either.

So the odds are that Laurenz's solution is what we are looking for.
Laurenz, Tom, any thoughts to share? I won't back-patch that ;)
--
Michael

Attachment Content-Type Size
win32-open-michael.patch text/x-diff 2.7 KB
win32-open-laurenz.patch text/x-diff 1.5 KB
win32-open-tom.patch text/x-diff 933 bytes

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Alexander Korotkov 2018-09-19 08:22:25 Re: pgsql: Add support for nearest-neighbor (KNN) searches to SP-GiST
Previous Message Alexander Korotkov 2018-09-18 22:56:58 pgsql: Add support for nearest-neighbor (KNN) searches to SP-GiST

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-09-19 03:06:47 Re: heap_sync seems rather oblivious to partitioned tables (wal_level=minimal)
Previous Message Thomas Munro 2018-09-19 01:58:36 Re: Usage of epoch in txid_current