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: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(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-06 03:46:28
Message-ID: CAJpy0uBSZcoVK+YUezoupcgB9y+pr1WvOz-0yBkCZrBatZzJEw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 5, 2026 at 11:46 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Aug 4, 2026 at 8:37 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > > Right, I've confirmed that this can't happen. I'm OK with keeping this code,
> > > since it's future-proof - otherwise, others might raise the same concern I
> > > imagined above.
> > >
> >
> > Okay, works for me, let's retain it. But good to change the comment to
> > indicate there is no such scenario at the moment, otherwise it may
> > confuse readers.
>
> Agreed.
>
> Please find the attached v8 patch.
>

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)

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?

thanks
Shveta

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-08-06 04:38:44 Re: Support for 8-byte TOAST values, round two
Previous Message Chao Li 2026-08-06 03:40:05 Re: Optimize UUID parse using SIMD