On Wed, Dec 20, 2023 at 12:46 PM Drouvot, Bertrand
<bertranddrouvot...@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
