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

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(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>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
Date: 2026-08-27 06:27:57
Message-ID: CAJpy0uC25xujScT02S7zRaBd5-tFgRD23dP3x2Gj7HW1tNTmKA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 25, 2026 at 3:59 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> 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.

I looked at the latest patch and the discussion upthread. I find this
solution of not dropping the temporary slots during a subxact error
much cleaner than the previous one. It gives us consistent behavior
across all cases and is easier to understand. There is also no
surprise for users, since this behavior is documented. But let's see
what others have to say here.

>
> 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 shveta malik 2026-08-27 06:31:08 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
Previous Message Masahiko Sawada 2026-08-27 06:27:15 Re: [PATCH] Release replication slot on error in SQL-callable slot functions