Thread: pgsql: Allow concurrent-safe open() and fopen() in frontend code forWi
Allow concurrent-safe open() and fopen() in frontend code for Windows PostgreSQL uses a custom wrapper for open() and fopen() which is concurrent-safe, allowing multiple processes to open and work on the same file. This has a couple of advantages: - pg_test_fsync does not handle O_DSYNC correctly otherwise, leading to false claims that disks are unsafe. - TAP tests can run into race conditions when a postmaster and pg_ctl open postmaster.pid, fixing some random failures in the buildfam. pg_upgrade is one frontend tool using workarounds to bypass file locking issues with the log files it generates, however the interactions with pg_ctl are proving to be tedious to get rid of, so this is left for later. Author: Laurenz Albe Reviewed-by: Michael Paquier, Kuntal Ghosh Discussion: https://postgr.es/m/1527846213.2475.31.camel@cybertec.at Discussion: https://postgr.es/m/16922.1520722108@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/0ba06e0bfb8cfd24ff17aca92aa72245ddd6c4d7 Modified Files -------------- 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(-)
Michael Paquier <michael@paquier.xyz> writes:
> Allow concurrent-safe open() and fopen() in frontend code for Windows
I'm surprised you didn't back-patch this --- isn't it a bug fix?
A compromise might be to push it to v11 but not further.
regards, tom lane
Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
From
Michael Paquier
Date:
On Fri, Sep 14, 2018 at 11:22:55AM -0400, Tom Lane wrote: > I'm surprised you didn't back-patch this --- isn't it a bug fix? > > A compromise might be to push it to v11 but not further. Yes, that's clearly a bug fix from pg_ctl point of view with TAP tests. However, I have rather cold feet about back-patching for two reasons as this introduces a behavior change: - The concurrency part disappears. - Caller of open() needs to enforce text mode to strip correctly CRLF characters. I would be fine to get that into v11 though, as that is not released yet. You will have to wait for a couple of days though, there is a long week-end here away from laptops and electronic devices ;) -- Michael
Attachment
Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
From
Michael Paquier
Date:
On Sat, Sep 15, 2018 at 07:49:39AM +0900, Michael Paquier wrote: > I would be fine to get that into v11 though, as that is not released > yet. You will have to wait for a couple of days though, there is a long > week-end here away from laptops and electronic devices ;) OK, REL_11_STABLE has been patched as well, after doing a couple of extra tests on Windows. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes: > OK, REL_11_STABLE has been patched as well, after doing a couple of > extra tests on Windows. BTW, I'm a bit concerned by the fact that bowerbird has failed its last couple of HEAD runs at the pgbench step. The first such failure was here: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2018-09-15%2014%3A19%3A58 Looking at the set of commits between the prior run and that one, it's hard to see anything that could have triggered the test failures other than this patch --- but I also don't see how this patch would've blown up pgbench without breaking earlier tests. Ideas? regards, tom lane
Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
From
Michael Paquier
Date:
On Mon, Sep 17, 2018 at 09:41:37AM -0400, Tom Lane wrote: > BTW, I'm a bit concerned by the fact that bowerbird has failed its > last couple of HEAD runs at the pgbench step. The first such > failure was here: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2018-09-15%2014%3A19%3A58 > > Looking at the set of commits between the prior run and that one, > it's hard to see anything that could have triggered the test failures > other than this patch --- but I also don't see how this patch would've > blown up pgbench without breaking earlier tests. Ideas? Thanks, I have been looking at the build farm but I missed this one. dory, which uses VS 2015 is not complaining because it does not run bincheck. At quick glance, it seems to be caused by process_file() in pgbench.c which would need to open files in text mode, and the input file parsing fails at the first '\' character found. I'll test that stuff on tomorrow morning manually. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Sep 17, 2018 at 09:41:37AM -0400, Tom Lane wrote:
>> Looking at the set of commits between the prior run and that one,
>> it's hard to see anything that could have triggered the test failures
>> other than this patch --- but I also don't see how this patch would've
>> blown up pgbench without breaking earlier tests. Ideas?
> Thanks, I have been looking at the build farm but I missed this one.
> dory, which uses VS 2015 is not complaining because it does not run
> bincheck. At quick glance, it seems to be caused by process_file() in
> pgbench.c which would need to open files in text mode, and the input
> file parsing fails at the first '\' character found.
Oh, you're thinking pgbench isn't robust against finding \r's visible
in its input? Could be.
> I'll test that stuff on tomorrow morning manually.
We've got a bit of a timing problem because we want to wrap 11beta4/rc1
(still TBD) in a few hours. I'll take a look and see if I can push a
quick fix before that.
regards, tom lane
Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
From
Andrew Dunstan
Date:
On 09/17/2018 10:48 AM, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> On Mon, Sep 17, 2018 at 09:41:37AM -0400, Tom Lane wrote: >>> Looking at the set of commits between the prior run and that one, >>> it's hard to see anything that could have triggered the test failures >>> other than this patch --- but I also don't see how this patch would've >>> blown up pgbench without breaking earlier tests. Ideas? >> Thanks, I have been looking at the build farm but I missed this one. >> dory, which uses VS 2015 is not complaining because it does not run >> bincheck. At quick glance, it seems to be caused by process_file() in >> pgbench.c which would need to open files in text mode, and the input >> file parsing fails at the first '\' character found. > Oh, you're thinking pgbench isn't robust against finding \r's visible > in its input? Could be. > >> I'll test that stuff on tomorrow morning manually. > We've got a bit of a timing problem because we want to wrap 11beta4/rc1 > (still TBD) in a few hours. I'll take a look and see if I can push a > quick fix before that. When you do I'll start a bowerbird run to check it. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 09/17/2018 10:48 AM, Tom Lane wrote:
>> We've got a bit of a timing problem because we want to wrap 11beta4/rc1
>> (still TBD) in a few hours. I'll take a look and see if I can push a
>> quick fix before that.
> When you do I'll start a bowerbird run to check it.
Pushed, please test.
I think there's a general issue here of exactly how we want pgwin32_fopen
to behave, but the immediate problem is best fixed by making pgbench deal
with Windows newlines more thoroughly.
regards, tom lane
Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
From
Andrew Dunstan
Date:
On 09/17/2018 12:13 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 09/17/2018 10:48 AM, Tom Lane wrote: >>> We've got a bit of a timing problem because we want to wrap 11beta4/rc1 >>> (still TBD) in a few hours. I'll take a look and see if I can push a >>> quick fix before that. >> When you do I'll start a bowerbird run to check it. > Pushed, please test. > > I think there's a general issue here of exactly how we want pgwin32_fopen > to behave, but the immediate problem is best fixed by making pgbench deal > with Windows newlines more thoroughly. > > Tests are still running, but it's past the pgbench stage on HEAD, so I think we're good. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 09/17/2018 12:13 PM, Tom Lane wrote:
>> Pushed, please test.
> Tests are still running, but it's past the pgbench stage on HEAD, so I
> think we're good.
I was more concerned about whether any of the post-pgbench steps would
show a failure; but it looks like HEAD's green, so probably v11 is too.
regards, tom lane