Re: Allow pg_archivecleanup to remove backup history files

From: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: michael(at)paquier(dot)xyz, masao(dot)fujii(at)oss(dot)nttdata(dot)com, bharath(dot)rupireddyforpostgres(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allow pg_archivecleanup to remove backup history files
Date: 2023-06-19 02:24:29
Message-ID: 3fe4af5a0a81fc6a2ec01cb484c0a487@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2023-06-16 11:22, Kyotaro Horiguchi wrote:
> At Thu, 15 Jun 2023 21:38:28 +0900, torikoshia
> <torikoshia(at)oss(dot)nttdata(dot)com> wrote in
>> On 2023-06-15 15:20, Kyotaro Horiguchi wrote:
>> Thanks for your review!
>> > + printf(_(" -x, --strip-extension=EXT strip this extention before
>> > identifying files fo clean up\n"));
>> > + printf(_(" -?, --help show this help, then exit\n"));
>> > After this change, some of these lines corss the boundary of the 80
>> > columns width. (is that policy viable noadays? I am usually working
>> > using terminal windows with such a width..) It's somewhat unrelated to
>> > this patch, but a help line a few lines further down also exceeds the
>> > width. We could shorten it by removing the "/mnt/server" portion, but
>> > I'm not sure if it's worth doing.
>>
>> I also highlight 80th column according to the wiki[1].
>> Since usage() in other files like pg_rewind.c and initdb.c also
>> exceeded the 80th column, I thought that was something like a guide.
>
> I think the page is suggesting about program code, not the messages
> that binaries print.

Thanks, now I understand what you meant.

> ASAICS the main section of the "pg_rewind --help" fits within 80
> columns. However, "initdb --help" does output a few lines exceeding
> the 80-column limit. Judging by the surrounding lines, I believe we're
> still aiming to maintain these the given width. I think we need to fix
> initdb in that regard.

Hmm, it seems some other commands also exceeds 80 columns:

pg_amcheck:
--skip=OPTION do NOT check "all-frozen" or
"all-visible" blocks
--startblock=BLOCK begin checking table(s) at the given
block number
--endblock=BLOCK check table(s) only up to the given
block number

--no-synchronized-snapshots do not use synchronized snapshots in
parallel jobs

pg_isready:
-t, --timeout=SECS seconds to wait when attempting connection, 0
disables (default: 3)

pg_receivewal:
--create-slot create a new replication slot (for the slot's
name see --slot)
--drop-slot drop the replication slot (for the slot's name
see --slot)

If you don't mind, I'm going to create another thread about this point.
I'll also discuss below line since it's unrelated to current thread
as you pointed out:

| pg_archivecleanup /mnt/server/archiverdir
000000010000000000000010.00000020.backup

Attached patch fixes the number of columns per row exceeding 80 by
changing to use getopt_long.

On 2023-06-16 11:30, Kyotaro Horiguchi wrote:
> At Fri, 16 Jun 2023 11:22:31 +0900 (JST), Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote in
>> ASAICS the main section of the "pg_rewind --help" fits within 80
>> columns. However, "initdb --help" does output a few lines exceeding
>> the 80-column limit. Judging by the surrounding lines, I believe we're
>> still aiming to maintain these the given width. I think we need to fix
>> initdb in that regard.
>
> Mmm, the message was introduced in 2012 by 8a02339e9b. I haven't
> noticed this until now...
>
> regards.

--
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION

Attachment Content-Type Size
v8-0001-Introduce-pg_archivecleanup-into-getopt_long.patch text/x-diff 4.0 KB
v8-0002-Allow-pg_archivecleanup-to-remove-backup-history-.patch text/x-diff 10.7 KB
v8-0003-Refactored-to-reduce-the-nesting-level.patch text/x-diff 5.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message James Coleman 2023-06-19 02:36:24 path->param_info only set for lateral?
Previous Message Masahiko Sawada 2023-06-19 01:19:24 Re: Subscription statistics are not dropped at DROP SUBSCRIPTION in some cases