At Fri, 27 Oct 2023 09:51:43 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in
> On Fri, Oct 27, 2023 at 9:37 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > IIUC the only possible way to reach this error (according to the
> > comment preceding it) is by the user overriding the GUC value (which
> > was defaulted -1) on the command line.
> >
>
> Yeah, this is my understanding as well.
>
> > + /*
> > + * The logical replication slots shouldn't be invalidated as
> > + * max_slot_wal_keep_size GUC is set to -1 during the upgrade.
> > + *
> > + * The following is just a sanity check.
> > + */
> >
> > Given that, I felt a more relevant msg/hint might be like:
> >
> > errmsg("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"),
> > errhint("Do not override \"max_slot_wal_keep_size\" using command line
> > options."));
> >
>
> But OTOH, we don't have a value of user-passed options to ensure that.
> So, how about a slightly different message: "This can be caused by
> overriding \"max_slot_wal_keep_size\" using command line options." or
> something along those lines? I see a somewhat similar message in the
> existing code (errhint("This can be caused ...")).
The suggested error message looks to me like that of the GUC
mechanism. While I don't have the wider picture about the feature,
might we consider rejecting the parameter setting? With that
modification, this message can be changed to elog one.
I believe it's somewhat inconsiderate to point out what shouldn't have
been done only after a problem has occurred.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center