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

On Wed, Sep 16, 2026 at 11:25 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Sep 10, 2026, at 06:21, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > 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
> > <v16-0001-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG19-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG18-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG17-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG16-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG15-Fix-replication-slot-leak-on-error-caught-in-a-s.patch><nocfbot-v16-0001-PG14-Fix-replication-slot-leak-on-error-caught-in-a-s.patch>
>
> I just reviewed v16 and have one concern.
>
> The comment explicitly says that temporary slots are left in place. For already-created temporary slots, that sounds reasonable. But what if the creation of a temporary slot fails within the subtransaction? For example:
> ```
> evantest=# DO $$
> evantest$# BEGIN
> evantest$# PERFORM pg_create_logical_replication_slot(
> evantest$# 'tmp_bad',
> evantest$# 'definitely_not_allowed',
> evantest$# true
> evantest$# );
> evantest$# EXCEPTION WHEN OTHERS THEN
> evantest$# RAISE NOTICE 'caught SQLSTATE %', SQLSTATE;
> evantest$# END
> evantest$# $$;
> NOTICE: caught SQLSTATE 42501
> DO
> evantest=#
> evantest=# SELECT slot_name,
> evantest-# plugin,
> evantest-# temporary,
> evantest-# active,
> evantest-# active_pid,
> evantest-# restart_lsn,
> evantest-# confirmed_flush_lsn,
> evantest-# catalog_xmin
> evantest-# FROM pg_replication_slots
> evantest-# WHERE slot_name = 'tmp_bad';
> slot_name | plugin | temporary | active | active_pid | restart_lsn | confirmed_flush_lsn | catalog_xmin
> -----------+------------------------+-----------+--------+------------+-------------+---------------------+--------------
> tmp_bad | definitely_not_allowed | t | t | 9668 | 0/01BFA5A8 | | 665
> (1 row)
> ```
>
> With a bad plugin, creation of the temporary slot fails, but the partially initialized slot remains after the error is caught. It remains until the session terminates, or it’s dropped explicitly. For a long-lived or pooled session, its restart_lsn continues to participate in ReplicationSlotsComputeRequiredLSN(), potentially causing unnecessary WAL retention.
>
> Therefore, should we distinguish a successfully created temporary slot from one whose creation is still in progress when the sub-transaction aborts, and drop the latter?

We had discussed this already, please see the email at [1] and the
responses to it. Since this issue is not new (it exists for other
slots too), it was decided to consider it separately on HEAD.

[1]: https://www.postgresql.org/message-id/CAJpy0uAwKM%3DLbnNp0rMevtCD9ub8zcADE9X1Z-PLwTmqFadgCQ%40mail.gmail.com

thanks
Shveta