From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output |
Date: | 2017-03-24 03:26:33 |
Message-ID: | CAB7nPqTUOpF792rDOnBkswZ=ZgHwxdB01OQU2tAF1KU4iUuLrw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Thu, Mar 23, 2017 at 1:10 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Andrew,
>
> * 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.
--
Michael
Attachment | Content-Type | Size |
---|---|---|
test-dump-nosync.patch | application/octet-stream | 14.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2017-03-24 03:53:05 | Re: pgsql: ICU support |
Previous Message | Tom Lane | 2017-03-24 03:19:10 | pgsql: Avoid syntax error on platforms that have neither LOCALE_T nor I |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-03-24 03:27:11 | Re: PATCH: pageinspect / add page_checksum and bt_page_items(bytea) |
Previous Message | Peter Eisentraut | 2017-03-24 03:21:39 | Re: [pgsql-www] Small issue in online devel documentation build |