Re: Function to get invalidation cause of a replication slot. - Mailing list pgsql-hackers

From shveta malik
Subject Re: Function to get invalidation cause of a replication slot.
Date
Msg-id CAJpy0uA4cj1vDVvPY=6jzsbTua2Rfo6NKeOctr72Pq2VJ=eoFw@mail.gmail.com
Whole thread Raw
In response to Re: Function to get invalidation cause of a replication slot.  ("Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>)
Responses Re: Function to get invalidation cause of a replication slot.
List pgsql-hackers
On Wed, Dec 20, 2023 at 12:46 PM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On 12/20/23 7:01 AM, shveta malik wrote:
> > Hello hackers,
> >
> > Attached is a patch which attempts to implement a new system function
> > pg_get_slot_invalidation_cause('slot_name')  to get invalidation cause
> > of a replication slot.
>
> Thanks! +1 for the idea to display this information through an SQL API.
>
> I wonder if we could add a new field to pg_replication_slots instead
> of creating a new function.
>
> > One of the use case scenarios for this function is another ongoing
> > thread "Synchronizing slots from primary to standby" at [1].  This
> > function is needed there to replicate invalidation-cause of a logical
> > replication slot from the primary server to the hot standby. But this
> > is an independent requirement in itself and thus makes sense to have
> > it implemented separately.
>
> Agree.
>
> My thoughts about it:
>
> 1 ===
>
> "Currently, users do not have a way to know the invalidation cause"
>
> I'm not sure about it, I think one could see the reasons in the log file.
>
> 2 ===
>
> "This function returns NULL if the replication slot is not found"
>
> Why not returning an error instead? (like pg_drop_replication_slot() does for example)
>
> FWIW, we'd not need to cover this case if the description would be added to pg_replication_slots.
>
> 3 ===
>
> +          <para>
> +           <literal>3</literal> = wal_level insufficient for the slot
> +          </para>
>
> wal_level insufficient on the primary maybe?
>
> 4 ===
>
> + * Returns ReplicationSlotInvalidationCause enum value for valid slot_name;
>
> I think it would be more user friendly to return a description instead of an enum value.
>
> 5 ===
>
>   doc/src/sgml/func.sgml              | 33 +++++++++++++++++++++++++++++++++
>   src/backend/replication/slotfuncs.c | 27 +++++++++++++++++++++++++++
>   src/include/catalog/pg_proc.dat     |  4 ++++
>
> Worth to add some coverage in 019_replslot_limit.pl and 035_standby_logical_decoding.pl?

I have recently added a test in 019_replslot_limit.pl in v2. Do you
suggest adding in 035_standby_logical_decoding as well? Will it have
additional benefits?

thanks
Shveta



pgsql-hackers by date:

Previous
From: Michail Nikolaev
Date:
Subject: Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements
Next
From: Bharath Rupireddy
Date:
Subject: Re: introduce dynamic shared memory registry