| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Temporary slot leak when creation fails in a subtransaction |
| Date: | 2026-09-30 09:40:15 |
| Message-ID: | CAJpy0uAnNgCaOpQcum2uqdq-4in_k7cQc0vqzVbyufeMfpwGCA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Sep 30, 2026 at 5:52 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Sep 29, 2026 at 2:10 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > Kuroda-san informed me off-list that it applies atop PG19 patch shared
> > in another thread. I will review.
>
> Thanks. The v1 patch attached here was built on top of the v19 patch
> from the other thread -
> https://postgr.es/m/CALj2ACXtHqVRBb40OHXx1JAMnv-L%3D-PFU_nyk2aB%3D0wMgkpGOw%40mail.gmail.com.
> Apologies for not being clear about that in the initial email.
>
> > But one question: are we planning to push this fix too to PG19 and
> > thus the patch is this way? And then later we can do it on HEAD, is
> > that the plan?
>
> Thanks Kuroda-san for sharing the resource owner (resowner) idea. But
> I don't think it fits well here. A slot can outlive the
> (sub)transaction that created it, in which case dropping it at the end
> of that (sub)transaction with a resowner might complicate things. It
> may not work for the walsender, which creates slots without a
> transaction. It also requires duplicating or moving some of the slot
> release code we have today in the shmem exit callback, sigsetjmp
> blocks etc. into the resowner callback. Also, logical decoding
> internally starts its own (sub)transactions, which makes the resowner
> approach a bit harder. IMHO, that feels more invasive than the fix
> needs, even on HEAD.
>
> So I still prefer the backend-local flag from v1, for both HEAD and
> the back branches. It also handles both logical and physical temporary
> slots, which a few of the other approaches mentioned upthread don't.
>
> While at it, I found another case. A persistent physical slot whose
> creation fails is also left behind, with or without a subtransaction,
> since release does not drop it.
yes, this is an existing issue as also raised by Sawada-san in [1].
> v1 does not handle that yet, but the
> same flag can cover it. Another approach is to create them as
> ephemeral first and persist them once creation succeeds, as logical
> slots do.
My preference even earlier was this approach: create them as EPHEMERAL
and persist them upon successful creation, but as Sawada-san
suggested, this should be done only on HEAD.
See 'Some hackers including me prefer option 1 but we agreed it
should be only master as it's impactful.' at [2]
So could it be that we use the static variable approach on back
branches but handle it more gracefully on HEAD using the EPHEMERAL
intermediate state? Or do you think that fix is not invasive and can
also be tried on back-branches?
> Although physical slot creation failures are rare, I think
> it is better to handle them as well to make the creation failure fixes
> complete.
I agree.
[1]: https://www.postgresql.org/message-id/CAD21AoC0UUHAWSA9TWBmBQkSt4v6QqhmE6HjNTVo55XgqTkG-g%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAD21AoDTAv8Py3q%3DJC5n3w%3DY1z-jU15DnU8d3dhP7JEwMZR2CQ%40mail.gmail.com
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alena Rybakina | 2026-09-30 09:42:33 | Re: pull-up subquery if JOIN-ON contains refs to upper-query |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-09-30 09:39:24 | RE: [PATCH] Add a check_hook for output_plugin_libraries |