public inbox for [email protected]  

help / color / mirror / Atom feed

From: shveta malik <[email protected]>
To: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
Date: Mon, 25 May 2026 15:28:26 +0530
Message-ID: <CAJpy0uBShUF_xm0=BVWivpWHt-4zs__k_3wL1RRjpi0Av8nsog@mail.gmail.com> (raw)
In-Reply-To: <CAHg+QDcf_9prAX=TaSO3UUiCLVD53bEw-KLqzAEXi+ud7h+Z4w@mail.gmail.com>
References: <CAHg+QDeuf9tCq3ce=kgFMJP0m=PZC+wi6B=yS+7V0vNXjLS31w@mail.gmail.com>
	<CAHGQGwFZaWj8DctXuhWQZwSqi631=NKzQJyDV4yqT1Qapt8MFQ@mail.gmail.com>
	<CALDaNm1Jjun=by60V-4EpLZe4pAKy0qVZ7ptyHGVCuDyKfo2xQ@mail.gmail.com>
	<CAHg+QDcu2x0mjkBSqRxP_8EQ6UmpuX_jMgdKLDkAL1=N6wzZCQ@mail.gmail.com>
	<CAJpy0uCmW_NUZN8mw26onvfoFzH_oMrFSKhLUhz896nDgf8c7Q@mail.gmail.com>
	<CAHg+QDcf_9prAX=TaSO3UUiCLVD53bEw-KLqzAEXi+ud7h+Z4w@mail.gmail.com>

On Mon, May 25, 2026 at 12:42 PM SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
>
> Hi
>
> On Fri, May 22, 2026 at 2:16 AM shveta malik <[email protected]> wrote:
>>
>> Thanks for reporting the issue. I could reproduce the same issue with
>> all these as well:
>>
>> pg_logical_slot_peek_changes
>> pg_logical_slot_get_binary_changes
>> pg_logical_slot_peek_binary_changes
>
>
> Please find the attached v2 patch that addressed these three cases as well.
>

Thank You for addressuing these cases. A few comments:

1)

+-- Test 2: session remains usable after the error (MyReplicationSlot cleared)

It shoudl be part of 'Test 1' itself and thus should not be named as 'Test 2'

2)
--------
+-- Test 4: copy_replication_slot with max_replication_slots exceeded.
+-- We reduce max_replication_slots artificially by filling all remaining slots.
+-- Instead, trigger an error by copying to an already-existing name.
+DO $$
+BEGIN
+    PERFORM pg_copy_logical_replication_slot('regression_slot_t3',
'regression_slot_t3');
+EXCEPTION WHEN OTHERS THEN
+    RAISE NOTICE 'caught: %', SQLERRM;
+END;
+$$;
+-- The original slot must still exist and be usable
+SELECT count(*) = 1 AS orig_slot_ok FROM pg_replication_slots
+    WHERE slot_name = 'regression_slot_t3';
-----------

I don't think we can hit the Assert with above test (at-least I could
not). Since creation of slot itself will fail as the slot with
same-name already exists, MyReplicationSlot will never be set and thus
Assert will not be hit.  A better testcase will be below which fails
during LoadOutputPlugin() after slot-creation and MyReplicationSlot is
set already.

SELECT pg_create_logical_replication_slot('src_slot', 'test_decoding');

DO $$
BEGIN
PERFORM pg_copy_logical_replication_slot('src_slot', 'dst_slot',
false, 'nonexistent_plugin');
EXCEPTION WHEN others THEN
RAISE NOTICE 'caught: %', SQLERRM;
END $$;

SELECT count(*) FROM pg_logical_slot_get_changes('src_slot', NULL, NULL);

3)
So overall these are the problematic APIs:

pg_create_logical_replication_slot
pg_replication_slot_advance
pg_copy_logical_replication_slot
pg_logical_slot_peek_binary_changes
pg_logical_slot_peek_changes
pg_logical_slot_get_changes
pg_logical_slot_get_binary_changes

First 3 are are mutually exclusive fixes fow which we have added
testcases. Last 4 are addressed by fixing common function
pg_logical_slot_get_changes_guts(). I think we should add a test case
for at-least any one of these APIs to cover
pg_logical_slot_get_changes_guts().

Thanks.
Shveta

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
  In-Reply-To: <CAJpy0uBShUF_xm0=BVWivpWHt-4zs__k_3wL1RRjpi0Av8nsog@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox