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

> 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?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/