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?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Reply via email to