agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Euler Taveira <euler@eulerto.com>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected changes of CurrentResourceOwner and CurrentMemoryContext
Date: Fri, 12 Sep 2025 08:46:18 +0200
Message-ID: <6242.1757659578@localhost> (raw)
In-Reply-To: <267a39fd-8b8a-48b0-8f99-b4257482ac95@app.fastmail.com>
References: <202509111800.kp6xfq6pnke7@alvherre.pgsql>
<267a39fd-8b8a-48b0-8f99-b4257482ac95@app.fastmail.com>
Euler Taveira <euler@eulerto.com> wrote:
> On Thu, Sep 11, 2025, at 3:05 PM, Álvaro Herrera wrote:
> > On 2025-Sep-03, Antonin Houska wrote:
> >
> >> When working on the REPACK command, we see an ERROR caused by unexpected
> >> change of CurrentResourceOwner [1]. I think the problem is that
> >> reorderbuffer.c does not restore the original value after calling
> >> RollbackAndReleaseCurrentSubTransaction(). The attached patch tries to handle
> >> the call like other callers throughout the tree do.
> >
>
> Interesting. I'm wondering that if this patch is applied we could remove the
> following code
>
> /*
> * Logical decoding could have clobbered CurrentResourceOwner during
> * transaction management, so restore the executor's value. (This is
> * a kluge, but it's not worth cleaning up right now.)
> */
> CurrentResourceOwner = old_resowner;
>
> from pg_logical_slot_get_changes_guts and LogicalSlotAdvanceAndCheckSnapState
> functions too. IIUC the referred code is a band-aid that will be improved
> someday.
Even though we're fixing the likely reason of this problem, we cannot be 100%
sure that no other problem like this still exists. So I'd not remove this
assignment. Maybe add Assert(CurrentResourceOwner == old_resowner) in front of
that, and adjust the comment?
> > I have registered this as
> > https://commitfest.postgresql.org/patch/6051/
> >
> > I've been wondering whether this should be backpatched. In principle
> > this is a bugfix, so it should, but I don't offhand recall any cases
> > where failure to set the current context/resowner in the other
> > reorderbuffer.c users causes a live bug, so ... maybe master only? I'm
> > wondering if it's possible where anybody _depends_ on the current
> > behavior, but I suppose that's quite unlikely.
> >
>
> I would say apply it to master only. If/when we have a bug report we can
> backpatch it.
+1
> Per the crash description, I'm not sure we can create a
> reproducible test case with the current supported commands. Am I wrong?
It seems so, at least with he "CurrentResourceOwner = old_resowner" assignment
in place. REPACK CONCURRENTLY exposes the problem a bit more because it has at
least one kind of resource open during logical decoding: relation.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
view thread (10+ messages) latest in thread
Message-ID: <6242.1757659578@localhost>
Permalink: ../6242.1757659578@localhost/
Also on: postgresql.org/message-id/6242.1757659578@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, euler@eulerto.com, alvherre@kurilemu.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected changes of CurrentResourceOwner and CurrentMemoryContext
In-Reply-To: <6242.1757659578@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox