At Thu, 9 Nov 2023 12:55:44 +1100, Peter Smith <smithpb2...@gmail.com> wrote in
> The most common pattern there is "You might need to increase %s.".
..
> Here is a patch to modify those other similar variations so they share
> that common wording.
>
> PSA.
I'm uncertain whether the phrases "Consider doing something" and "You
might need to do something" are precisely interchangeable. However,
(for me..) it appears that either phrase could be applied for all
messages that the patch touches.
In short, I'm fine with the patch.
By the way, I was left scratching my head after seeing the following
message.
> ereport(PANIC,
> (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
> - errmsg("could not find free replication state, increase
> max_replication_slots")));
Being told to increase max_replication_slots in a PANIC
message feels somewhat off to me. Just looking at the message, it
seems unconvincing to increase "slots" because there is a lack of
"state". So, I poked around in the code and found the following
comment:
> ReplicationOriginShmemSize(void)
> {
> ...
> /*
> * XXX: max_replication_slots is arguably the wrong thing to use, as here
> * we keep the replay state of *remote* transactions. But for now it seems
> * sufficient to reuse it, rather than introduce a separate GUC.
> */
I haven't read the related code, but if my understanding based on this
comment is correct, wouldn't it mean that a lack of storage space for
the state at the location outputting the message indicates a bug in
the program, not a user configuration error? In other words, isn't
this message something that at least shouldn't be a user-facing
message, and might it be more appropriate to use an Assert instead?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
