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

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.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com