From: Noah Misch <noah@leadboat.com>
To: Robert Haas <robertmhaas@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Smolkin Grigory <smallkeen@gmail.com>
Subject: Re: race condition in pg_class
Date: Wed, 12 Jun 2024 10:54:52 -0700
Message-ID: <20240612175452.de.nmisch@google.com> (raw)
In-Reply-To: <ZmfUgRBbyLIGoHyM@paquier.xyz>
References: <1617596.1698446455@sss.pgh.pa.us>
<20231027232612.99.nmisch@google.com>
<20231102030915.d3.nmisch@google.com>
<20240512232923.aa.nmisch@google.com>
<20240605181706.fe.nmisch@google.com>
<CA+TgmobfMU5pdXP36D5iAwxV5WKE_vuDLtp_1QyH+H5jMMt21g@mail.gmail.com>
<ZmJENQ1BiMoStBPD@paquier.xyz>
<CA+TgmoZ_Qezv0+AXN4s=OumQwvo9cf+1e+BA=y42WKth+QQK+A@mail.gmail.com>
<20240611021927.30.nmisch@google.com>
<ZmfUgRBbyLIGoHyM@paquier.xyz>
On Tue, Jun 11, 2024 at 01:37:21PM +0900, Michael Paquier wrote:
> On Mon, Jun 10, 2024 at 07:19:27PM -0700, Noah Misch wrote:
> > On Fri, Jun 07, 2024 at 09:08:03AM -0400, Robert Haas wrote:
> >> I think the core code should provide an "Injection Point" wait event
> >> type and let extensions add specific wait events there, just like you
> >> did for "Extension".
> >
> > Michael, could you accept the core code offering that, or not? If so, I am
> > content to implement that. If not, for injection point wait events, I have
> > just one priority. The isolation tester already detects lmgr locks without
> > the test writer teaching it about each lock individually. I want it to have
> > that same capability for injection points. Do you think we can find something
> > everyone can accept, having that property? These wait events show up in tests
> > only, and I'm happy to make the cosmetics be anything compatible with that
> > detection ability.
>
> Adding a wait event class for injection point is an interesting
> suggestion that would simplify the detection in the isolation function
> quite a bit. Are you sure that this is something that would be fit
> for v17 material? TBH, I am not sure.
If I were making a list of changes always welcome post-beta, it wouldn't
include adding wait event types. But I don't hesitate to add one if it
unblocks a necessary test for a bug present in all versions.
> At the end, the test coverage has the highest priority and the bugs
> you are addressing are complex enough that isolation tests of this
> level are a necessity, so I don't object to what
> inplace050-tests-inj-v2.patch introduces with the naming dependency
> for the time being on HEAD. I'll just adapt and live with that
> depending on what I deal with, while trying to improve HEAD later on.
Here's what I'm reading for each person's willingness to tolerate each option:
STRATEGY | Paquier | Misch | Haas
--------------------------------------------------------
new "Injection Point" wait type | maybe | yes | yes
INJECTION_POINT(...) naming | yes | yes | unknown
isolation spec says event names | yes | no | unknown
Corrections and additional strategy lines welcome. Robert, how do you judge
the lines where I've listed you as "unknown"?
> I'm still wondering if there is something that could be more elegant
> than a dedicated class for injection points, but I cannot think about
> something that would be better for isolation tests on top of my head.
> If there is something I can think of, I'll just go and implement it :)
I once considered changing them to use advisory lock waits instead of
ConditionVariableSleep(), but I recall that was worse from the perspective of
injection points in critical sections.
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: noah@leadboat.com, robertmhaas@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, smallkeen@gmail.com
Subject: Re: race condition in pg_class
In-Reply-To: <20240612175452.de.nmisch@google.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox