Re: [PATCH] Release replication slot on error in SQL-callable slot functions

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
Date: 2026-08-04 11:16:48
Message-ID: CAJpy0uA6FqAMcHD3CNQRy+tiUxdBTKoJLz=Pr2ijxM36zx4bhw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 4, 2026 at 4:26 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tuesday, August 4, 2026 4:39 PM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > On Friday, July 31, 2026 3:12 AM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > I read the issue, patches and comments so far and here's my take on it.
> > >
> > > ...
> >
> > Thanks for sharing the patch.
> >
> > IIUC, the patch only handles releasing the slot when the subtransaction aborts
> > after ReplicationSlotCreate(), but it doesn't address the original repro[1]
>
> Sorry, I misread the code and missed the new logic in ReplicationSlotAcquire().
> It does fix all the issues. Please ignore above.
>
> The only thing I notice is that this new design seems to touch more scope than
> the original PG_TRY/PG_CATCH approach, since it releases the slot not only on
> ERROR but also on a manual transaction abort (a direct AbortCurrentTransaction()
> call without an intervening ERROR). It also seems slightly inconsistent that we
> do this for subtransactions but not for top-level transactions, but maybe it's
> OK as it only targets to fix the PL/pgSQL EXCEPTION case.
>
> One interesting case I thought of: we currently record
> GetCurrentSubTransactionId() when creating or acquiring a slot, and that ID is a
> logical subxid (starting from 1). So it looks possible for the following to
> happen: the user acquires the slot in a subtransaction with subxid 2 and commits
> the whole transaction; then, in a new transaction, the user starts a
> subtransaction that also gets subxid 2 and aborts it.

IIUC, you are referring to the case which Bharath and myself discussed
in [1]. See [1] and previous few emails.

> In that case the slot
> would be released, even though the aborted subtransaction is a different one
> from the subtransaction that originally acquired the slot.

Even if that happens, I think it is covered because on Commit, patch
changes `acquiredInSubId` to parent-Id and thus a new subxid 2 will
not be releasing it.

> I think the HEAD
> cannot create such a case using SQL APIs, so it might not be a serious issue,

Right.

> but just share it for reference. Maybe some comments are needed to hint user
> about the risk of this.
>

[1]: https://www.postgresql.org/message-id/CAJpy0uBOVFzEda4Z6a2tLNNb34EMme9Rc8zFyVSXGM0vxibgzQ%40mail.gmail.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2026-08-04 11:26:02 Re: Bug in asynchronous Append
Previous Message Vitaly Davydov 2026-08-04 11:16:24 Re: Deadlock detector fails to activate on a hot standby replica