public inbox for [email protected]
help / color / mirror / Atom feedFrom: SATYANARAYANA NARLAPURAM <[email protected]>
To: shveta malik <[email protected]>
Cc: Fujii Masao <[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: Tue, 9 Jun 2026 21:06:29 -0700
Message-ID: <CAHg+QDfZV_U2SQDrGy7DyBFtb45rQ_4Ln2NiQBk3p74bFTj40Q@mail.gmail.com> (raw)
In-Reply-To: <CAJpy0uD-_VurCWEYe3mczoZFDpXqjaZmC8gygaNZ5a_cuObOdQ@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>
<CAJpy0uD-_VurCWEYe3mczoZFDpXqjaZmC8gygaNZ5a_cuObOdQ@mail.gmail.com>
Hi
On Thu, May 28, 2026 at 10:45 PM shveta malik <[email protected]>
wrote:
> 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.
Got stuck with something. Will send a revised patch over the weekend, in
the meantime if you want to take it forward please feel free to.
Thanks,
Satya
>
>
>
view thread (23+ messages) latest in thread
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: <CAHg+QDfZV_U2SQDrGy7DyBFtb45rQ_4Ln2NiQBk3p74bFTj40Q@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