Re: [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output

From: Noah Misch <noah(at)leadboat(dot)com>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output
Date: 2017-04-05 06:49:41
Message-ID: 20170405064941.GB2702846@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers 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(at)snowman(dot)net> wrote:
> > * Andrew Dunstan (andrew(dot)dunstan(at)2ndquadrant(dot)com) wrote:
> >> On 03/22/2017 11:39 AM, Stephen Frost wrote:
> >> > * Andrew Dunstan (andrew(at)dunslane(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2017-04-05 13:03:46 pgsql: dblink: Small code rearrangement for clarity
Previous Message Peter Eisentraut 2017-04-05 04:39:25 pgsql: Capitalize names of PLs consistently

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2017-04-05 06:55:18 Re: extended stats not friendly towards ANALYZE with subset of columns
Previous Message Noah Misch 2017-04-05 06:47:55 Re: Query fails when SRFs are part of FROM clause (Commit id: 69f4b9c85f)