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

On Sat, Aug 29, 2026 at 4:19 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Thu, Aug 27, 2026 at 9:19 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > 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.
>
> While I agree that it would be an ideal behavior and the analysis
> holds for logical slots, I want to note that persistent physical
> replication slots are created with RS_PERSISTENT so if an error
> happens during the slot creation the slot is left behind. Also,
> logical persistent slots actually have the same gap: if
> ReplicationSlotPersist() raises an error it leaves a persistent slot
> behind as well. Given that slot creation and drop are not
> transactional operations, and that leaving a slot behind on a failure
> is not a new behavior, I'm inclined toward only releasing the slot at
> the subxact abort. We can discuss the better behavior on HEAD
> separately.
>

Okay, fair enough. I agree.

thanks
Shveta