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

On Sat, Aug 29, 2026 at 4:42 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> In the replication slot subxact callback, on the abort path, we need
> to know whether the slot's creation failed. Ephemeral slots already
> handle that, but only for persistent logical slots. A temporary slot
> stays RS_TEMPORARY throughout. So there are a few ways to solve this:
>
> 1/ Also mark temporary slots as ephemeral initially and transition
> them to RS_TEMPORARY once creation succeeds. A quick check shows this
> needs changes in many places.
> 2/ Introduce a new state to represent a temporary slot still in
> creation (RS_TEMPORARY_EPHEMERAL or such).
> 3/ Use a boolean in the ReplicationSlot structure
> (is_create_in_progress or such), and in the subxact callback, when the
> slot is temporary and is_create_in_progress is set, drop just that
> temporary slot and leave the others alone.
>
> I prefer option 3,
>

I would prefer option-1 as we have a similar pre-existing behaviour.

to keep it simple without adding a new state, and
> because it is back-branch friendly.
>

BTW, I was thinking this to be improved in HEAD-only as it is a more
impactful change.

> >
> > 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. 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.
>
> Hi Amit, By restricting in the code, does that mean adding an Assert,
> or a WARNING, or a WARNING plus slot release (not an error), in the
> replication slot subxact callback on the commit path, instead of
> handing the slot off to the parent across the subtransaction boundary?
>

Yes, I would prefer WARNING similar to existing cases for resource
leaks in commit paths. One example of a similar existing case is:
------
/* Complain if any allocated files remain open at commit. */
if (isCommit && numAllocatedDescs > 0)
elog(WARNING, "%d temporary files and directories not closed at
end-of-transaction",
numAllocatedDescs);
-------

Based on above, I am imagining a check/WARNING on lines of:
-------
if (isCommit)
{
/*
* The subxact that acquired the slot is committing
with the slot still
* held. No slot function does that today. Warn, and
hand the slot to
* the parent so it is still released if an ancestor aborts.
*/
Assert(MyReplicationSlot != NULL);
ereport(WARNING,
(errcode(ERRCODE_WARNING),
errmsg("subtransaction left
replication slot \"%s\" acquired",

NameStr(MyReplicationSlot->data.name)),
errhint("Check for missing
\"ReplicationSlotRelease\" calls.")));

MyReplicationSlotSubId = parentSubid;
return;
}
--------

--
With Regards,
Amit Kapila.