Re: [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output - Mailing list pgsql-hackers

From Noah Misch
Subject Re: [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output
Date
Msg-id 20170405064941.GB2702846@tornado.leadboat.com
Whole thread Raw
In response to Re: [HACKERS] [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers
On Fri, Mar 24, 2017 at 12:26:33PM +0900, Michael Paquier wrote:
> On Thu, Mar 23, 2017 at 1:10 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Andrew Dunstan (andrew.dunstan@2ndquadrant.com) wrote:
> >> On 03/22/2017 11:39 AM, Stephen Frost wrote:
> >> > * Andrew Dunstan (andrew@dunslane.net) wrote:
> >> >> Sync pg_dump and pg_dumpall output
> >> > This probably should have adjusted all callers of pg_dump in the
> >> > regression tests to use the --no-sync option, otherwise we'll end up
> >> > spending possibly a good bit of time calling fsync() during the
> >> > regression tests unnecessairly.
> >>
> >> All of them? The imnpact is not likely to be huge in most cases
> >> (possibly different on Windows). On crake, the bin-check stage actually
> >> took less time after the change than before, so I suspect that the
> >> impact will be pretty small.
> >
> > Well, perhaps not all, but I'd think --no-sync would be better to have
> > in most cases.  We turn off fsync for all of the TAP tests and all
> > initdb calls, so it seems like we should here too.  Perhaps it won't be
> > much overhead on an unloaded system, but not all of the buildfarm
> > animals seem to be on unloaded systems, nor are they particularly fast
> > in general or have fast i/o..
> 
> Agreed.
> 
> >> Still I agree that we should have tests for both cases.
> >
> > Perhaps, though if I recall correctly, we've basically had zero calls
> > for fsync() until this.  If we don't feel like we need to test that in
> > the backend then it seems a bit silly to feel like we need it for
> > pg_dump's regression coverage.
> >
> > That said, perhaps the right answer is to have a couple tests for both
> > the backend and for pg_dump which do exercise the fsync-enabled paths.
> 
> And agreed.
> 
> The patch attached uses --no-sync in most places for pg_dump, except
> in 4 places of 002_pg_dump.pl to stress as well the sync code path.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Andrew,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com



pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: Query fails when SRFs are part of FROM clause (Commit id:69f4b9c85f)
Next
From: Noah Misch
Date:
Subject: Re: extended stats not friendly towards ANALYZE with subset ofcolumns