Re: Introduce XID age and inactive timeout based replication slot invalidation

On Tue, Feb 4, 2025 at 4:02 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Mon, Feb 3, 2025 at 5:34 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Feb 3, 2025 at 6:16 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > >
> > > 2.
> > > + appendStringInfo(&err_hint, "You might need to increase \"%s\".",
> > > + "max_slot_wal_keep_size");
> > > break;
> > > 2a.
> > > In this case, shouldn't you really be using macro _("You might need to
> > > increase \"%s\".") so that the common format string would be got using
> > > gettext()?
> > >
> > > ~
> > >
> > >
> > > ~~~
> > >
> > > 3.
> > > + appendStringInfo(&err_hint, "You might need to increase \"%s\".",
> > > + "idle_replication_slot_timeout");
> > > + break;
> > >
> > > 3a.
> > > Ditto above. IMO this common format string should be got using macro.
> > > e.g.: _("You might need to increase \"%s\".")
> > >
> > > ~
> >
> > Instead, we can directly use '_(' in errhint as we are doing in one
> > other similar place "errhint("%s", _(view_updatable_error))));". I
> > think we didn't use it for errdetail because, in one of the cases, it
> > needs to use ngettext
> >
>
> -1 for this suggestion because this will end up causing a gettext() on
> the entire hint where the GUC has already been substituted.
>
> e.g. it is effectively doing
>
> _("You might need to increase \"max_slot_wal_keep_size\".")
> _("You might need to increase \"idle_replication_slot_timeout\".")
>
> But that is contrary to the goal of reducing the burden on translators
> by using *common* messages wherever possible. IMO we should only
> request translation of the *common* part of the hint message.
>

Fair point. So, we can ignore my suggestion.

--
With Regards,
Amit Kapila.