From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dipesh Pandit <dipesh.pandit@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Jeevan Ladhe <jeevanladhe.os@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>
Cc: Abhijit Menon-Sen <ams@toroid.org>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org, tushar <tushar.ahuja@enterprisedb.com>
Subject: Re: refactoring basebackup.c (zstd workers)
Date: Sun, 20 Mar 2022 14:40:50 -0500
Message-ID: <20220320194050.GX28503@telsasoft.com> (raw)
In-Reply-To: <CA+TgmoaaDXOT+vFDcPrnXjPTU7oFPY2_pkeQFTdSGJfrqO2LNQ@mail.gmail.com>
References: <20220312015253.zokhkwdn5xbbd4vf@alap3.anarazel.de>
<CA+TgmoZ10QPYaUxe8+RfJT5ptyDe-ML=oE_FyyFa+sNS2GBgZw@mail.gmail.com>
<CAN1g5_F0KBgitEVLO8T0ynaBcQBfRMCjH9wKguDJZpX1hF3gFA@mail.gmail.com>
<20220314163547.GT28503@telsasoft.com>
<CA+TgmoaYrNxoGB8x_7Fk5uKXfbyMVAdSXTnfQN-1zu_3gB_v=Q@mail.gmail.com>
<20220314171152.GU28503@telsasoft.com>
<CA+TgmoYfV1ASq0QVR1QHaJnXfr+fPLoPByQL1SfWqjUuKtX_UA@mail.gmail.com>
<CA+TgmoYvpetyRAbbg1M8b3-iHsaN4nsgmWPjOENu5-doHuJ7fA@mail.gmail.com>
<20220317194130.GM28503@telsasoft.com>
<CA+TgmoaaDXOT+vFDcPrnXjPTU7oFPY2_pkeQFTdSGJfrqO2LNQ@mail.gmail.com>
On Sun, Mar 20, 2022 at 03:05:28PM -0400, Robert Haas wrote:
> On Thu, Mar 17, 2022 at 3:41 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > - errmsg("unrecognized compression algorithm: \"%s\"",
> > + errmsg("unrecognized compression algorithm \"%s\"",
> >
> > Most other places seem to say "compression method". So I'd suggest to change
> > that here, and in doc/src/sgml/ref/pg_basebackup.sgml.
>
> I'm not sure that's really better, and I don't think this patch is
> introducing an altogether novel usage. I think I would probably try to
> standardize on algorithm rather than method if I were standardizing
> the whole source tree, but I think we can leave that discussion for
> another time.
The user-facing docs are already standardized using "compression method", with
2 exceptions, of which one is contrib/ and the other is what I'm suggesting to
make consistent here.
$ git grep 'compression algorithm' doc
doc/src/sgml/pgcrypto.sgml: Which compression algorithm to use. Only available if
doc/src/sgml/ref/pg_basebackup.sgml: compression algorithm is selected, or if server-side compression
> > + result->parse_error =
> > + pstrdup("found empty string where a compression option was expected");
> >
> > Needs to be localized with _() ?
> > Also, document that it's pstrdup'd.
>
> Did the latter. The former would need to be fixed in a bunch of places
> and while I'm happy to accept an expert opinion on exactly what needs
> to be done here, I don't want to try to do it and do it wrong. Better
> to let someone with good knowledge of the subject matter patch it up
> later than do a crummy job now.
I believe it just needs _("foo")
See git grep '= _('
I mentioned another issue off-list:
pg_basebackup.c:2741:10: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
2741 | Assert(compressloc = COMPRESS_LOCATION_SERVER);
| ^~~~~~~~~~~
pg_basebackup.c:2741:3: note: in expansion of macro ‘Assert’
2741 | Assert(compressloc = COMPRESS_LOCATION_SERVER);
This crashes the server using your v2 patch:
src/bin/pg_basebackup/pg_basebackup --wal-method fetch -Ft -D - -h /tmp --no-sync --no-manifest --compress=server-zstd:level, |wc -c
I wonder whether the syntax should really use both ":" and ",".
Maybe ":" isn't needed at all.
This patch also needs to update the other user-facing docs.
typo: contain a an
--
Justin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, robertmhaas@gmail.com, dipesh.pandit@gmail.com, andres@anarazel.de, jeevanladhe.os@gmail.com, alvherre@alvh.no-ip.org, noriyoshi.shinoda@hpe.com, ams@toroid.org, 9erthalion6@gmail.com, jeevan.ladhe@enterprisedb.com, mark.dilger@enterprisedb.com, tushar.ahuja@enterprisedb.com
Subject: Re: refactoring basebackup.c (zstd workers)
In-Reply-To: <20220320194050.GX28503@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox