On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi
<horikyota....@gmail.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
