public inbox for [email protected]
help / color / mirror / Atom feedFrom: shveta malik <[email protected]>
To: Fujii Masao <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: vignesh C <[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: Fri, 29 May 2026 11:15:25 +0530
Message-ID: <CAJpy0uD-_VurCWEYe3mczoZFDpXqjaZmC8gygaNZ5a_cuObOdQ@mail.gmail.com> (raw)
In-Reply-To: <CAHGQGwEbt2UdAsA4i8aE0snysmEtG0YeRtpr-NO=uTtJa4AB-Q@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>
<CAJpy0uBShUF_xm0=BVWivpWHt-4zs__k_3wL1RRjpi0Av8nsog@mail.gmail.com>
<CAHg+QDf5PVyFgesBNs1GvOnuk_khoXifo96A7QW1EJ8zhhBxyw@mail.gmail.com>
<CAJpy0uCCqFLY7pu0RQVcS9fRr0FimFMuHPsMBQ-KzEGX3BEGPA@mail.gmail.com>
<CAJpy0uDHMvpUAdwXA3X7ugmO8S7kry-ZtrKUcugpX3WWp8hykw@mail.gmail.com>
<CAHg+QDdEGGQrmQXwH0_Y+DRe_tx5jxv9K+7vpCnooQn2_6QykA@mail.gmail.com>
<CAJpy0uBo-OuzyZZ=LY8L48Udt8=1Mffh2rrpXaTbKK_F9sJx7A@mail.gmail.com>
<CAHg+QDeKC=_31Fvs2pOVkJCdkpNuoJmLmXV5hOApStpODYWsXw@mail.gmail.com>
<CAHGQGwEoENMBTu0=f2h5_GFfTc-dGEo+_CfS0jinPwTX2AcyYA@mail.gmail.com>
<CAJpy0uB3sb2wfXaoYd8XnK53GcpTLW6BJLJve_K6JdNy8-dpgg@mail.gmail.com>
<CAHGQGwEbt2UdAsA4i8aE0snysmEtG0YeRtpr-NO=uTtJa4AB-Q@mail.gmail.com>
On Wed, May 27, 2026 at 5:40 PM Fujii Masao <[email protected]> wrote:
>
> On Wed, May 27, 2026 at 8:00 PM shveta malik <[email protected]> wrote:
> > pg_copy_physical_replication_slot() should not have it as the common
> > 'copy_replication_slot' is already fixed in the patch.
>
> copy_replication_slot() calls create_physical_replication_slot() before
> entering the PG_TRY/PG_CATCH block. So if create_physical_replication_slot()
> throws an error, wouldn't the same issue still occur?
>
You are right. Using v5, if I force create_physical_replication_slot()
to fail while executing pg_copy_physical_replication_slot() (through
debugging), I can see that the next slot-related call hits an Assert.
1)
I also noticed that for pg_copy_logical_replication_slot(), we do not
hit the CATCH block of copy_replication_slot(), but instead the one in
create_logical_replication_slot(). That behavior seems fine.
However, I noticed some inconsistencies in the implementation.
For create_physical_replication_slot(), the caller
pg_create_physical_replication_slot() contains the TRY-CATCH block,
whereas create_logical_replication_slot() contains its own TRY-CATCH
block internally.
I think it makes more sense to keep the TRY-CATCH handling inside the
internal functions, i.e. create_logical_replication_slot() and
create_physical_replication_slot(), since that would automatically
cover all callers. For example, create_logical_replication_slot() is
invoked from multiple places, so callers need not worry about cleanup
handling themselves. Similarly, for
create_physical_replication_slot(), we could move the TRY-CATCH block
inside the function instead of having it in
pg_create_physical_replication_slot(). Doing so would also resolve the
issue with pg_copy_physical_replication_slot().
2)
I also feel that ReplicationSlotCreate() should be moved inside the
TRY block in create_logical_replication_slot().
Currently, if in the future ReplicationSlotCreate() gains any
post-slot-creation implementation that could throw an error, we may
end up leaving the system in an unsafe state. Keeping it inside the
TRY block would make the code more robust against such future changes.
~~
Reveiwign further. Need to review few more things on v5/v6.
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: <CAJpy0uD-_VurCWEYe3mczoZFDpXqjaZmC8gygaNZ5a_cuObOdQ@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