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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
Date: 2026-08-24 22:29:00
Message-ID: CALj2ACUuAgDJ3kCXJHNnV3R=ziZvuGHiBH8zfcT1wJg5+q=Pww@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Fri, Aug 21, 2026 at 3:34 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I think we need to carefully think about whether we drop all temp
> slots at subxact abort and when we do that.
>
> Also, is calling ReplicationSlotCleanup() (and possibly
> ReplicationSlotRelease()) during sub-transaction abort really safe in
> the first place? That function could raise errors and we call it out
> of transactions elsewhere.
>
> I'm inclined to think AtEOSubXact_ReplicationSlot() should only
> release the slot and not call ReplicationSlotCleanup() at all. That
> said, I'm not fully convinced of this either as it might be
> inconsistent with top-level error cases in a sense.

Upon thinking about this more, I realize that temporary slot handling
in the subxact error callback looks complicated and hard to reason
about from the end user's perspective. I checked commit a924c327e27
that introduced temporary slots. It seems like the behavior was kept
simple: release at session end or upon error, without classifying
whether the error came from slot handling or something else. Subxact
errors caught by exception blocks are usually there to continue in the
same transaction, not like top-level ERRORs. I think leaving temp slot
cleanup out of subxact error handling and slightly modifying the docs
seems fine.

On whether it is safe to release the slot during subxact abort, that
function could raise an error in one rare scenario (a file rename
failure). I checked that when an error is thrown while releasing the
slot invoked from PostgresMain()'s sigsetjmp handler, it re-enters
that handler, and the resulting recursion eventually overflows the
stack, causing a PANIC. The subxact abort path gets the same behavior
when releasing the slot there.

In short, having just the slot release in the subxact path gives the
same error behavior, is simple to reason about, and fixes the crash
reported in this thread.

Thoughts?

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2026-08-24 22:30:46 Re: Preserve statistics targets with ALTER TABLE ALTER COLUMN TYPE
Previous Message Melanie Plageman 2026-08-24 22:28:41 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions