Re: Track in pg_replication_slots the reason why slots conflict?

Hi,

On 2023-12-21 16:08:48 +0530, shveta malik wrote:
> On Thu, Dec 21, 2023 at 3:10 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > Hi,
> >
> > On 2023-12-21 09:21:04 +0900, Michael Paquier wrote:
> > > While listening at Bertrand's talk about logical decoding on standbys
> > > last week at Prague, I got surprised by the fact that we do not
> > > reflect in the catalogs the reason why a conflict happened for a slot.
> > > There are three of them depending on ReplicationSlotInvalidationCause:
> > > - WAL removed.
> > > - Invalid horizon.
> > > - Insufficient WAL level.
> >
> > It should be extremely rare to hit any of these other than "WAL removed", so
> > I'm not sure it's worth adding interface complexity to show them.
> >
> >
> > > ReplicationSlotCtl holds this information, so couldn't it be useful
> > > for monitoring purposes to know why a slot got invalidated and add a
> > > column to pg_get_replication_slots()? This could just be an extra
> > > text conflicting_reason, defaulting to NULL when there's nothing to
> > > see.
> >
> > Extra columns aren't free from a usability perspective. IFF we do something, I
> > think it should be a single column with a cause.
>
> Thanks for the feedback. But do you mean that we replace existing
> 'conflicting' column with 'cause' in both the function and view
> (pg_get_replication_slots() and pg_replication_slots)? Or do you mean
> that we expose 'cause' from pg_get_replication_slots() and use that to
> display 'conflicting' in pg_replication_slots view?

I'm not entirely sure I understand the difference - just whether we add one
new column or replace the existing 'conflicting' column? I can see arguments
for either. A conflicting column where NULL indicates no conflict, and other
values indicate the reason for the conflict, doesn't seem too bad.

> And if we plan to return/display cause from either function or view,
> then shall it be enum 'ReplicationSlotInvalidationCause' or
> description/text corresponding to enum?

We clearly can't just expose the numerical value for a C enum. So it has to be
converted to something SQL representable.

> In the other feature being discussed "Synchronize slots from primary
> to standby" [1] , there is a requirement to replicate invalidation
> cause of slot from the primary to standby and thus it is needed in
> enum form there. And thus there was a suggestion earlier to have the
> function return enum-value and let the view display it as
> text/description to the user. So kindly let us know your thoughts.
>
> [1] - https://www.postgresql.org/message-id/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

Can you point me to a more specific message for that requirement? It seems
pretty odd to me. Your link goes to the top of a 400 message thread, I don't
have time to find one specific design point in that...

Greetings,

Andres