Re: Introduce XID age and inactive timeout based replication slot invalidation - Mailing list pgsql-hackers

On Thu, Nov 28, 2024 at 5:20 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Nisha. Here are some review comments for patch v51-0002.
>
> ======
> src/backend/replication/slot.c
>
> ReplicationSlotAcquire:
>
> 2.
> GENERAL.
>
> This just is a question/idea. It may not be feasible to change. It
> seems like there is a lot of overlap between the error messages in
> 'ReplicationSlotAcquire' which are saying "This slot has been
> invalidated because...", and with the other function
> 'ReportSlotInvalidation' which is kind of the same but called in
> different circumstances and with slightly different message text. I
> wondered if there is a way to use common code to unify these messages
> instead of having a nearly duplicate set of messages for all the
> invalidation causes?
>

The error handling could be moved to a new function; however, as you
pointed out, the contexts in which these functions are called differ.
IMO, a single error message may not suit both cases. For example,
ReportSlotInvalidation provides additional details and a hint in its
message, which isn’t necessary for ReplicationSlotAcquire.
Thoughts?

--
Thanks,
Nisha