Hi, On Thu, Aug 6, 2026 at 5:26 AM Ashutosh Sharma <[email protected]> wrote: > > > Please find the attached v9 patch. > > The patch looks good overall - just a few quick comments:
Thanks for taking a look at it.
> After releasing the slot in AtEOSubXact_ReplicationSlot(), I'd suggest
> adding these assertions:
>
> Assert(MyReplicationSlot == NULL);
> Assert(acquiredInSubId == InvalidSubTransactionId);
The slot release function sets MyReplicationSlot to NULL in both the
ephemeral and the other path, and clears acquiredInSubId with no early
return, so both conditions already hold there. I would prefer not to
add asserts that re-check what the release just above guarantees.
> + acquiredInSubId = GetCurrentSubTransactionId();
>
> Since the entire cleanup logic relies on the value of acquiredInSubId,
> should we make this more robust, something like:
>
> acquiredInSubId = IsTransactionState()
> ? GetCurrentSubTransactionId()
> : InvalidSubTransactionId;
I think it's redundant. When not in a transaction,
GetCurrentSubTransactionId already returns an invalid subxact ID (all
the commit and abort paths set it to invalid), so this check doesn't
add anything IMO.
> + if (isCommit)
> + {
> + acquiredInSubId = parentSubid;
> + return;
> + }
>
> Is there a test case covering this subtransaction handoff scenario? For
> example:
>
> DO $$
> BEGIN
> BEGIN
> PERFORM pg_replication_slot_advance('slot', some_valid_lsn);
> EXCEPTION WHEN division_by_zero THEN
> NULL;
> END;
>
> RAISE EXCEPTION 'outer failure';
> EXCEPTION WHEN OTHERS THEN
> NULL;
> END $$;
There can be the following cases:
1/ The inner subxact acquires the slot and the advance succeeds. It
releases the slot before returning, so there is no handoff.
2/ The advance errors out. Either the exception matches and the error
is caught, or it doesn't match and is re-thrown. In both cases the
slot is released when the inner subxact aborts, because PL/pgSQL rolls
back the subxact before checking for a matching handler. So by the
time a re-thrown error reaches the outer level, the slot is already
gone.
So the handoff branch is never reached today. It only fires if a
subxact commits while still holding a slot, which no slot function
does, since they all release before returning (see the discussion on
it upthread:
https://www.postgresql.org/message-id/CALj2ACUD_K5zBgXD3ebYmhmouJx91fq%2BaiLeD8HSuC6xnYvj3g%40mail.gmail.com).
I realized the comment on the exception not matching needed some
adjustment. I made that change and attached the v10 patch.
BTW, here's the CF entry for this bug:
https://commitfest.postgresql.org/patch/7097/.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
v10-0001-Fix-replication-slot-leak-on-error-caught-in-a-s.patch
Description: Binary data
