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

On Thu, Aug 27, 2026 at 6:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Aug 27, 2026 at 11:58 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, Aug 24, 2026 at 3:29 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > >
> > > 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.
> >
>
> True, but OTOH, won't we already clean up resources not directly
> associated with subxact in AtEOSubXact_LargeObject() or
> AtEOSubXact_Files()? I don't see any problem as far as the current
> pattern of usage for slots.

I agree.

> The new restriction this patch will add is
> "a slot acquired in a subxact does not survive that subxact being
> unwound." which should be okay because of its similarity with
> top-level xact behavior. I feel if possible we should restrict such
> usage explicitly in code in some way rather than one finding out this
> as a surprise.
>
> *
> An error raised and caught in a
> + subtransaction, for example by a
> + <application>PL/pgSQL</application> exception block, does not drop
> + them.
>
> Based on above, something like below won't clean up temp slots and end
> up holding xmin.
> DO $$ BEGIN
> PERFORM pg_create_logical_replication_slot('s', 'nonexistent_plugin', true);
> EXCEPTION WHEN OTHERS THEN RAISE NOTICE '%', SQLERRM;
> END $$;

Well, on rethinking, I feel that if we encounter an error while
creating a slot, whether persistent or temporary, the slot should be
dropped right there.

This already works correctly for persistent slots: by the time the
slot reaches ReplicationSlotRelease, it is still in RS_EPHEMERAL state
and is therefore dropped by release. OTIOH, a temporary slot is left
behind. I think the temporary slot should also be dropped because the
caller never received a reference to it. I don't see a legitimate use
case where a temp slot should survive specifically because its
creation call failed.

The second case is an 'error while using an already-existing slot'
(temporary or persistent). I think it is fine to let the temporary
slot survive (similar to a persistent slot) in this case, consistent
with what we've documented: the caller has a reference to the temp
slot and can continue using it even after a caught subtransaction
failure. Thoughts?

> The situation can be worse if one retries creating the slot in the
> exception handler with a different name.

If an error occurs during creation, I think the slot should be dropped.

thanks
Shveta