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

On Fri, Jul 31, 2026 at 12:43 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Jun 16, 2026 at 4:28 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I've reviewed the v6 patch, and here are some comments:
>
> Thanks Satya for the off-list discussion on this.
>
> Thanks Sawada-san and others for reviewing it.
>
> I read the issue, patches and comments so far and here's my take on it.
>
> When an error reaches the top-level sigsetjmp in PostgresMain(), it
> releases the acquired replication slot and cleans up all the
> session-level temp slots. However, when a subtransaction (subxact)
> catches the error in its own PG try-catch block without re-throwing
> it, that never happens, and the acquired slot can be left behind,
> leaving the session-level MyReplicationSlot pointing to an unreleased
> slot. In assert builds this causes a failure. In release builds it
> silently leaves a dangling slot, retaining WAL and blocking vacuum due
> to catalog_xmin. Any SQL-callable replication slot function can hit
> this (except pg_sync_replication_slots(), which already releases the
> slot on error via PG_ENSURE_ERROR_CLEANUP()), especially when called
> inside a PL/pgSQL, PL/Perl, PL/Python or PL/Tcl exception block, which
> under the hood is implemented as subxact with a PG try-catch block in
> more or less the same way. This happens on all supported branches,
> back to PG14.
>
> Here's the flow in each case:
>
> Transaction: PostgresMain()'s sigsetjmp -> slot function. An error in
> the slot function reaches PostgresMain()'s sigsetjmp, which releases
> the slot.
>
> Subxact: PostgresMain()'s sigsetjmp -> exec_stmts's sigsetjmp -> slot
> function. An error in the slot function inside the subxact gets to
> exec_stmts's sigsetjmp, which doesn't re-throw the error and so never
> reaches PostgresMain()'s sigsetjmp, leaving the slot behind.
>
> To fix this, we need to ensure the slot acquired in the subxact also
> gets released in the error paths. There are a couple of ways of fixing
> it. See below.
>
> > Maybe pg_sync_replication_slots() has the same problem if it's called
> > inside an exception block?
>
> No, it already wraps the code that runs with the slot acquired in
> PG_ENSURE_ERROR_CLEANUP(), which releases the slot on error. I
> verified this locally with an injection point.
>
> > The patch adds PG_TRY()/PG_CATCH() to each replication slot function,
> > but there is no comment explaining why we need them even though we
> > call ReplicationSlotRelease() and ReplicationSlotCleanup() in error
> > paths. Also, while probably the proposed idea works for back branches,
> > I guess we might want to consider more comprehensive approach for HEAD
> > to deal with this issue as it seems to me very error-prone,
> > especially when adding a new replication slot function.
>
> This approach wraps each SQL-callable slot function in its own PG
> try-catch that releases the slot on error and re-throws, as proposed
> in the earlier patches. The flow then becomes PostgresMain()'s
> sigsetjmp -> exec_stmts's sigsetjmp -> slot function's sigsetjmp ->
> slot function. An error gets to the slot function's own sigsetjmp
> first, which releases the slot and re-throws, so by the time it
> reaches exec_stmts's sigsetjmp the slot is already released.
>
> Although it looks simple and keeps the changes localized to the slot
> functions, it has to be repeated in every such function and any future
> one, so the change is spread across many callsites, adding a PG
> try-catch to each. That gets error-prone as new slot functions are
> added and is a bit hard to maintain long-term.
>
> > This problem stems from the fact that when replication slots are used
> > within an exception block, we don't reach the error path even if an
> > error occurs, and we cannot simply call ReplicationSlotRelease()
> > during subtransaction abort as we need to start and abort transaction
> > while holding a slot. But I guess that we can release the slot when
> > aborting the subtransaction or above (sub)transaction where we
> > created/acquired the slot. I've drafted the patch for this idea and
> > confirmed it passes the regression tests, I still need to verify its
> > feasibility though.
>
> Thanks for the POC, and the attached patch builds on it.
>
> It releases the slot on subxact abort, at the subxact where it was
> acquired, instead of in each function. It records the subxact id that
> acquired the slot and, at every subxact end, checks whether that
> subxact is the one ending. On abort of the acquiring subxact it
> releases the slot. On commit it hands the id to the parent, so the
> slot is still released if the parent, or an ancestor, later aborts. A
> subxact id is used rather than a nesting level, since levels are
> reused.
>
> It does touch the subxact abort path, which is shared by all
> transactions, so it needs some care to not release the slot that
> logical decoding holds. As you noted, decoding starts and aborts an
> internal transaction (or subxact) per decoded transaction while
> holding the slot. That always happens below the acquiring subxact, so
> matching on the acquiring subxact id leaves those alone.
>
> > + /* XXX need to call ReplicationSlotCleanup() as well? */
>
> Releasing a temporary slot does not drop it, so it would keep
> retaining WAL and blocking vacuum until session end. So on abort,
> after releasing the held slot, the patch also drops the session's
> temporary slots the same way the top-level path does. This runs only
> when the aborting subxact held a slot, so a caught error that held no
> slot does not drop them, unlike a top-level error which always does. I
> left an XXX comment in the patch with more details on this.
>
> Please have a look at the attached v7 patch.
>

Hi Bharath, Sawana-San,
I definitely like this approach better. I'll review it and verify it
in more depth soon.

thanks
Shveta