From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | bharath(dot)rupireddyforpostgres(at)gmail(dot)com, amit(dot)kapila16(at)gmail(dot)com, alvherre(at)alvh(dot)no-ip(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: A recent message added to pg_upgade |
Date: | 2023-11-02 03:25:53 |
Message-ID: | CAHut+PsMzqd-nH6XYyNLNuHgSoz8jCK3YmzWiLpBtDbcGxkuEw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 2, 2023 at 1:58 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Thanks you for the comments!
>
> At Wed, 1 Nov 2023 18:08:19 +1100, Peter Smith <smithpb2250(at)gmail(dot)com> wrote in
> > Hi, here are some minor review comments for the v3 patch.
> >
> > ======
> > src/backend/access/transam/xlog.c
>
...
> > 2.
>
> > + GUC_check_errdetail("\"max_slot_wal_keep_size\" must be set to -1
> > during binary upgrade mode.");
>
> > Some of the other GUC_check_errdetail()'s do not include the GUC name
> > in the translatable message text. Isn't that a preferred style?
>
> > SUGGESTION
> > GUC_check_errdetail("\"%s\" must be set to -1 during binary upgrade mode.",
> > "max_slot_wal_keep_size");
>
> I believe that that style was adopted to minimize translatable
> messages by consolidting identical ones that only differ in variable
> names. I see both versions in the tree. I didn't find necessity to
> adopt this approach for this specific message, especially since I'm
> skeptical about adding new messages that end with "must be set to -1
> during binary upgrade mode". (pg_upgrade sets synchronous_commit,
> fsync and full_page_writes to "off".)
>
> However, some unique messages are in this style, so I'm fine with
> using that style. Revised accordingly.
>
Checking this patch yesterday prompted me to create a new thread
questioning the inconsistencies of the "GUC names in messages". In
that thread, Tom Lane replied and gave some background information [1]
about the GUC name embedding versus substitution. In hindsight, I
think your original message was fine as-is, but there seem to be
examples of every kind of style, so whatever you do would have some
precedent.
The patch v4 LGTM.
======
[1] https://www.postgresql.org/message-id/2758485.1698848717%40sss.pgh.pa.us
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2023-11-02 03:32:07 | Re: A recent message added to pg_upgade |
Previous Message | torikoshia | 2023-11-02 03:24:50 | Re: pg_rewind WAL segments deletion pitfall |