Horton

Thread

Commits

  1. Add --no-sync to pg_upgrade's uses of pg_dump and pg_dumpall.

  2. Sync pg_dump and pg_dumpall output

  1. add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Nathan Bossart <nathandbossart@gmail.com> — 2024-05-03T17:13:48Z

    This is likely small potatoes compared to some of the other
pg_upgrade-related improvements I've proposed [0] [1] or plan to propose,
but this is easy enough, and I already wrote the patch, so here it is.
AFAICT there's no reason to bother syncing these dump files to disk.  If
someone pulls the plug during pg_upgrade, it's not like you can resume
pg_upgrade from where it left off.  Also, I think we skipped syncing before
v10, anyway, as the --no-sync flag was only added in commit 96a7128, which
added the code to sync dump files, too.

[0] https://postgr.es/m/20240418041712.GA3441570%40nathanxps13
[1] https://postgr.es/m/20240503025140.GA1227404%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  2. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Peter Eisentraut <peter@eisentraut.org> — 2024-05-08T08:09:46Z

    On 03.05.24 19:13, Nathan Bossart wrote:
> This is likely small potatoes compared to some of the other
> pg_upgrade-related improvements I've proposed [0] [1] or plan to propose,
> but this is easy enough, and I already wrote the patch, so here it is.
> AFAICT there's no reason to bother syncing these dump files to disk.  If
> someone pulls the plug during pg_upgrade, it's not like you can resume
> pg_upgrade from where it left off.  Also, I think we skipped syncing before
> v10, anyway, as the --no-sync flag was only added in commit 96a7128, which
> added the code to sync dump files, too.

Looks good to me.

  3. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Nathan Bossart <nathandbossart@gmail.com> — 2024-05-08T18:38:50Z

    On Wed, May 08, 2024 at 10:09:46AM +0200, Peter Eisentraut wrote:
> On 03.05.24 19:13, Nathan Bossart wrote:
>> This is likely small potatoes compared to some of the other
>> pg_upgrade-related improvements I've proposed [0] [1] or plan to propose,
>> but this is easy enough, and I already wrote the patch, so here it is.
>> AFAICT there's no reason to bother syncing these dump files to disk.  If
>> someone pulls the plug during pg_upgrade, it's not like you can resume
>> pg_upgrade from where it left off.  Also, I think we skipped syncing before
>> v10, anyway, as the --no-sync flag was only added in commit 96a7128, which
>> added the code to sync dump files, too.
> 
> Looks good to me.

Thanks for looking.  I noticed that the version check is unnecessary since
we always use the new binary's pg_dump[all], so I removed that in v2.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  4. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-05-08T18:49:58Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> Thanks for looking.  I noticed that the version check is unnecessary since
> we always use the new binary's pg_dump[all], so I removed that in v2.

+1

			regards, tom lane

  5. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Michael Paquier <michael@paquier.xyz> — 2024-05-09T00:03:56Z

    On Wed, May 08, 2024 at 02:49:58PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> Thanks for looking.  I noticed that the version check is unnecessary since
>> we always use the new binary's pg_dump[all], so I removed that in v2.
> 
> +1

+1.  Could there be an argument in favor of a backpatch?  This is a
performance improvement, but one could also side that the addition of
sync support in pg_dump[all] has made that a regression that we'd
better fix because the flushes don't matter in this context.  They
also bring costs for no gain.
--
Michael

  6. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Nathan Bossart <nathandbossart@gmail.com> — 2024-05-09T19:34:25Z

    On Thu, May 09, 2024 at 09:03:56AM +0900, Michael Paquier wrote:
> +1.  Could there be an argument in favor of a backpatch?  This is a
> performance improvement, but one could also side that the addition of
> sync support in pg_dump[all] has made that a regression that we'd
> better fix because the flushes don't matter in this context.  They
> also bring costs for no gain.

I don't see a strong need to back-patch this, if for no other reason than
it seems to have gone unnoticed for 7 major versions.  Plus, based on my
admittedly limited testing, this is unlikely to provide significant
improvements.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  7. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-09T19:58:37Z

    > On 9 May 2024, at 21:34, Nathan Bossart <nathandbossart@gmail.com> wrote:
> 
> On Thu, May 09, 2024 at 09:03:56AM +0900, Michael Paquier wrote:
>> +1.  Could there be an argument in favor of a backpatch?  This is a
>> performance improvement, but one could also side that the addition of
>> sync support in pg_dump[all] has made that a regression that we'd
>> better fix because the flushes don't matter in this context.  They
>> also bring costs for no gain.
> 
> I don't see a strong need to back-patch this, if for no other reason than
> it seems to have gone unnoticed for 7 major versions.  Plus, based on my
> admittedly limited testing, this is unlikely to provide significant
> improvements.

Agreed, this is a nice little improvement but it's unlikely to be enough of a
speedup to warrant changing the backbranches.

--
Daniel Gustafsson

  8. Re: add --no-sync to pg_upgrade's calls to pg_dump and pg_dumpall

    Nathan Bossart <nathandbossart@gmail.com> — 2024-07-01T15:22:59Z

    Committed.

-- 
nathan