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

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.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com