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

On Mon, Aug 24, 2026 at 3:29 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Fri, Aug 21, 2026 at 3:34 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I think we need to carefully think about whether we drop all temp
> > slots at subxact abort and when we do that.
> >
> > Also, is calling ReplicationSlotCleanup() (and possibly
> > ReplicationSlotRelease()) during sub-transaction abort really safe in
> > the first place? That function could raise errors and we call it out
> > of transactions elsewhere.
> >
> > I'm inclined to think AtEOSubXact_ReplicationSlot() should only
> > release the slot and not call ReplicationSlotCleanup() at all. That
> > said, I'm not fully convinced of this either as it might be
> > inconsistent with top-level error cases in a sense.
>
> Upon thinking about this more, I realize that temporary slot handling
> in the subxact error callback looks complicated and hard to reason
> about from the end user's perspective. I checked commit a924c327e27
> that introduced temporary slots. It seems like the behavior was kept
> simple: release at session end or upon error, without classifying
> whether the error came from slot handling or something else. Subxact
> errors caught by exception blocks are usually there to continue in the
> same transaction, not like top-level ERRORs. I think leaving temp slot
> cleanup out of subxact error handling and slightly modifying the docs
> seems fine.

Yeah, it makes sense to drop temp slots when an error reaches the top
level: the transaction is gone, the error is reported to the client
and the session goes back to idle, so nothing is left running that
could still be using the slot. A subtransaction abort is different --
the transaction continues, and when an exception block catches the
error it is never reported at all, so the application may well still
be using a temporary slot it created earlier.

> On whether it is safe to release the slot during subxact abort, that
> function could raise an error in one rare scenario (a file rename
> failure). I checked that when an error is thrown while releasing the
> slot invoked from PostgresMain()'s sigsetjmp handler, it re-enters
> that handler, and the resulting recursion eventually overflows the
> stack, causing a PANIC. The subxact abort path gets the same behavior
> when releasing the slot there.
>
> In short, having just the slot release in the subxact path gives the
> same error behavior, is simple to reason about, and fixes the crash
> reported in this thread.

One thing I'm a bit concerned about is that this would be the first
caller to invoke ReplicationSlotRelease() from inside the transaction
machinery. Today the call in PostgresMain() runs after
AbortCurrentTransaction() has returned, and WalSndErrorCleanup()
likewise. An error escaping AbortSubTransaction() does not recover:
the subxact is left on the stack, the next AbortCurrentTransaction()
runs the abort again, and it ends in PANIC. So
ReplicationSlotRelease() must not fail there.

As far as I read the code, it doesn't. For a non-ephemeral slot it
only takes the slot's spinlock and ProcArrayLock and updates some
ReplicationSlot fields. For an ephemeral slot it drops the slot, and
ReplicationSlotDropPtr() is already careful about that: it downgrades
a rename() failure to WARNING for non-persistent slots. So calling
ReplicationSlotRelease() during a subtransaction abort looks safe to
me.

Regards,

--
Masahiko Sawada

Amazon Web Services: https://aws.amazon.com