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

Hi,

On Tue, Sep 8, 2026 at 9:24 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> > There can be two cases for external modules implementing logical
> > decoding functionality. A function that unknowingly forgets to call
> > ReplicationSlotRelease(), and a function that intentionally holds the
> > slot across subxact boundaries and releases it later in the top-level
> > transaction. For example
> >
> > ```
> > BeginInternalSubTransaction("xxx");
> > ReplicationSlotAcquire(name, ...);
> > <do something>
> > ReleaseCurrentSubTransaction();
> > <do more something>
> > ReplicationSlotRelease();
> > ```
> >
> > The above seems like a legitimate usage (though we don't know if there
> > is any real user of this pattern today). We can't easily distinguish
> > between the two cases in the subxact commit path. The first case is
> > more of a coding and reviewing problem. In both cases, calling the
> > function twice in a row would hit Assert(MyReplicationSlot == NULL) or
> > silently overwrite the slot, but the intentional case must already be
> > aware of this. Even if the core emits a WARNING and users report it,
> > there may not be anything we can do about it. If they release the slot
> > at the end of the function, it is not a problem. If they forget, they
> > need to fix it themselves.
> >
> > Given all this, emitting a WARNING on a subxact commit may not seem
> > right even on HEAD. Silently handing off the slot to the parent
> > transaction on subxact commit seems like the better approach.
> >
>
> I agree there could be such a scenario in the future, especially since
> we don't document or define a rule that a slot must be released in the
> same subtransaction where it was acquired. Even if no existing user
> exposed slot-function does this today, an extension could.
>
> But I feel there should be at least some way to signal that there's a
> chance of a slot leak, for the cases where it actually is one. How
> about putting in a DEBUG message noting that the slot was retained
> across a subxact boundary? Something like:
>
> elog(DEBUG1,
> "replication slot \"%s\" acquired in subtransaction retained
> across its commit; ownership transferred to parent",
> NameStr(MyReplicationSlot->data.name));

Upon thinking more and discussing off-list with Amit and Sawada-san,
here is what I have. In the PG20+ branches, I added a WARNING and
removed the assert while handing off the slot across subtransaction
boundaries during commits. We do not know if there are any such
legitimate uses, but if there are, those users would get the WARNING
reported. On HEAD it is easier to remove the WARNING later if it feels
annoying for such users. In the backbranches,
AtEOSubXact_ReplicationSlot() is a no-op for commits because the
WARNING may not be a good idea there, and we do not have a good use
case for it on commits anyway. Hope this simplifies the fix.

I used similar wording to the above for the WARNING.

Please find the attached v16 patches prepared for all the supported branches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com