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

Hi,

On Wed, Aug 5, 2026 at 8:46 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> Thanks Bharath. A few trivial comments:
>
> 1)
> + * We must not get here while decoding is running. Decoding starts and
> + * aborts an internal (sub)transaction while holding the slot, for each
> + * decoded transaction (ReorderBufferProcessTXN()) and when executing
> + * invalidations (ReorderBufferImmediateInvalidation()), but that always
> + * happens below the acquiring subxact, so those aborts have a different
> + * (deeper) id and do not match here. Decoding also runs with a historic
> + * snapshot set up, so assert that it is not.
>
> It is slightly difficult to understand this comment. What does 'below'
> mean? Shall we rephrase 'but that always...' to:
>
> However, those subtransactions are always nested below the subtransaction that
> acquired the slot, so their subtransaction IDs are deeper and therefore do not
> match here. Decoding also ....
>
> (I hope your comment meant this, else let me know)

That's right. TXN -> SUBTXN1 (acquires the slot) -> SUBTXN2 (internal
subxact started during decoding in ReorderBufferProcessTXN()), and
while in SUBTXN2, the historic snapshot is held. Your wording looks
fine to me.

> 2)
> +-- Test 3: same as Test 1 for a temporary slot. Releasing a temporary slot on
> +-- error does not drop it, so it would keep holding back WAL removal and the
> +-- catalog xmin. The session's temporary slots are dropped as well, so none is
> +-- left behind.
>
> The comment is slightly confusing. We are intititally saying 'it does
> not drop temp-slot' and then saying 'it is dropped'. Do we want to
> distinguish the sentences as old and post-patch behaviour somehow?

I wanted to say the difference between slot release and cleanup there.
I simplified it as follows, and the comments in
AtEOSubXact_ReplicationSlot() have a detailed explanation anyway.

+-- Test 3: same as Test 1 for a temporary slot, which is dropped rather than
+-- just released, so it is not left behind after the error.

Please find the attached v9 patch.

I verified that the same issue reported here exists all the way back
to PG14. I want to backport it, since it is a bug that can be
reproduced with simple SQL queries by the end user and can cause slot
leaks and vacuum issues. What do you think?

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