public inbox for [email protected]
help / color / mirror / Atom feedFrom: Sami Imseih <[email protected]>
To: Ayush Tiwari <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Andres Freund <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored procedures with pg_stat_statements and track_functions =
Date: Wed, 17 Jun 2026 15:26:33 -0500
Message-ID: <CAA5RZ0tuiNhkL30Juxwcox0C6pBCX6H-EVXZoRRQc4X6g6HTxQ@mail.gmail.com> (raw)
In-Reply-To: <CAJTYsWWJEchycUwkt6=Hk3j3ROOJKVp2n5yaU7JNqwqOmyoEMA@mail.gmail.com>
References: <[email protected]>
<CAJTYsWVqtWF+KJUdoKrzUM9QPQA5qD225AoeBeN7cwT4V1qd6A@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAJTYsWWJEchycUwkt6=Hk3j3ROOJKVp2n5yaU7JNqwqOmyoEMA@mail.gmail.com>
> This patch is very close to what Sami has posted on his PGSS thread,
> v3-0002, using a missing_ok instead of a skip_dropped:
> https://www.postgresql.org/message-id/[email protected]...
> I didn't suspect that we would need something like that for a
> backpatch, but well.
Right, my intention was just adding infrastructure to make tolerating
a dropped entry possible and to be used by an extension in the future.
But, this bug report is timely and now it looks like we need this for
race conditions that are possible in core.
> A couple of things which I'm not clear about (these are not blockers
> just questions for my understanding):
>
> - With the check moved into the wrapper, pgstat_drop_entry_internal()
> still keeps its own "already dropped" elog(). Every path into
> _internal now seems to guarantee the entry isn't dropped, so
> _internal's copy looks unreachable after the patch
> ,and it's the one with the richer refcount/generation detail.
Right. Michael's approach of moving the ERROR into the wrapper is better
than keeping it in _internal. Since after the patch no caller enters
pgstat_drop_entry_internal() with a dropped entry
(pgstat_drop_database_and_contents() and pgstat_drop_matching_entries()
already filtered them out, and now the wrapper does too)
> Was the idea to leave it as a backstop, or would folding the handling into
> one place (or making _internal's an Assert) be cleaner?
The check in _internal should be converted to an Assert. This documents
that callers must only pass "live" entries, which will be the case
for all callers after
the patch
> - In the missing_ok path the wrapper returns true, so the post-commit
> caller skips the not_freed_count++/GC request that a "real" not-freed
> drop would do. That seems harmless since the entry self-heals
> but was returning true there a deliberate choice over mirroring
> the not-freed/false path? I need to take a look again at this, maybe
> I missed something.
Finding an already dropped entry tells me that the first caller to drop the
entry also triggered a gc request, so we should not request it again.
--
Sami Imseih
Amazon Web Services (AWS)
view thread (17+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored procedures with pg_stat_statements and track_functions =
In-Reply-To: <CAA5RZ0tuiNhkL30Juxwcox0C6pBCX6H-EVXZoRRQc4X6g6HTxQ@mail.gmail.com>
* 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