Re: race condition in pg_class

Noah Misch <noah@leadboat.com>

From: Noah Misch <noah@leadboat.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>, Smolkin Grigory <smallkeen@gmail.com>
Date: 2024-06-12T19:32:23Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Replace tests of ALTER DATABASE RESET TABLESPACE.

  2. meson: Flush stdout in testwrap

  3. Fix catcache invalidation of a list entry that's being built

  4. Stop reading uninitialized memory in heap_inplace_lock().

  5. Fix use of uninitialized value in previous commit.

  6. Back-patch "Refactor code in tablecmds.c to check and process tablespace moves"

  7. Fix data loss at inplace update after heap_update().

  8. For inplace update durability, make heap_update() callers wait.

  9. Warn if LOCKTAG_TUPLE is held at commit, under debug_assertions.

  10. Don't lose partitioned table reltuples=0 after relhassubclass=f.

  11. Fix new assertion for MERGE view_name ... DO NOTHING.

  12. Remove configuration-dependent output from new inplace-inval test.

  13. AccessExclusiveLock new relations just after assigning the OID.

  14. Cope with inplace update making catcache stale during TOAST fetch.

  15. Expand comments and add an assertion in nodeModifyTable.c.

  16. Improve test coverage for changes to inplace-updated catalogs.

  17. Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.

  18. Lock owned sequences during ALTER TABLE SET { LOGGED | UNLOGGED }.

  19. Make TAP todo_start effects the same under Meson and prove_check.

  20. Add an injection_points isolation test suite.

  21. Add wait event type "InjectionPoint", a custom type like "Extension".

  22. Create waitfuncs.c for pg_isolation_test_session_is_blocked().

  23. Rework planning and execution of UPDATE and DELETE.

On Wed, Jun 12, 2024 at 02:08:31PM -0400, Robert Haas wrote:
> On Wed, Jun 12, 2024 at 1:54 PM Noah Misch <noah@leadboat.com> wrote:
> > 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.
> 
> However, injection points themselves are not present in all versions,
> so even if we invent a new wait-event type, we'll have difficulty
> testing older versions, unless we're planning to back-patch all of
> that infrastructure, which I assume we aren't.

Right.  We could put the injection point tests in v18 only instead of v17+v18.
I feel that would be an overreaction to a dispute about names that show up
only in tests.  Even so, I could accept that.

> Personally, I think the fact that injection point wait events were put
> under Extension is a design mistake that should be corrected before 17
> is out of beta.

Works for me.  I don't personally have a problem with the use of Extension,
since it is a src/test/modules extension creating them.

> > 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'd tolerate INJECTION_POINT() if we had no other option but I think
> it's clearly inferior. Does the last line refer to putting the
> specific wait event names in the isolation spec file? If so, I'd also
> be fine with that.

Yes, the last line does refer to that.  Updated table:

STRATEGY                        | Paquier | Misch | Haas
--------------------------------------------------------
new "Injection Point" wait type | maybe   | yes   | yes
INJECTION_POINT(...) naming     | yes     | yes   | no
isolation spec says event names | yes     | no    | yes

I find that's adequate support for the first line.  If there are no objections
in the next 24hr, I will implement that.