Re: A recent message added to pg_upgade

On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Hello.
>
> Some messages recently introduced by commit 29d0a77fa6 seem to deviate
> slightly from our standards.
>
> + if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
> + {
> + ereport(ERROR,
> + errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("replication slots must not be invalidated during the upgrade"),
> + errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"));
>
> The message for errhint is not a complete sentence.

Yeah, the hint message should have ended with a period -
https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION.

> The second attached does this.
>
> What do you think about those?

+ errmsg("replication slot is invalidated during upgrade"),
+ errhint("Set \"max_slot_wal_keep_size\" to -1 to
avoid invalidation."));
}

The above errhint LGTM. How about a slightly different errmsg, like
the following?

+ errmsg("cannot invalidate replication slots when
in binary upgrade mode"),
+ errhint("Set \"max_slot_wal_keep_size\" to -1 to
avoid invalidation."));

".... when in binary upgrade mode" is being used in many places.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com