Re: race condition in pg_class

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Noah Misch <noah@leadboat.com>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>, Smolkin Grigory <smallkeen@gmail.com>
Date: 2024-06-16T00:28:05Z
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 Thu, Jun 13, 2024 at 07:42:25PM -0700, Noah Misch wrote:
> On Fri, Jun 14, 2024 at 09:58:59AM +0900, Michael Paquier wrote:
>> GetWaitEventCustomIdentifier() is incorrect, and should return
>> "InjectionPoint" in the default case of this class name, no?
> 
> I intentionally didn't provide a default event ID for InjectionPoint.
> PG_WAIT_EXTENSION needs a default case for backward compatibility, if nothing
> else.  For this second custom type, it's needless complexity.  The value
> 0x0B000000U won't just show up like PG_WAIT_EXTENSION does.
> GetLWLockIdentifier() also has no default case.  How do you see it?

I would add a default for consistency as this is just a few extra
lines, but if you feel strongly about that, I'm OK as well.  It makes
a bit easier the detection of incorrect wait event numbers set
incorrectly in extensions depending on the class wanted.

> The patch added to xfunc.sgml an example of using it.  I'd be more inclined to
> delete the WaitEventExtensionNew() docbook documentation than to add its level
> of detail for WaitEventInjectionPointNew().  We don't have that kind of
> documentation for most extension-facing C functions.

It's one of the areas where I think that we should have more
documentation, not less of it, so I'd rather keep it and maintaining
it is not really a pain (?).  The backend gets complicated enough
these days that limiting what developers have to guess on their own is
a better long-term approach because the Postgres out-of-core ecosystem
is expanding a lot (aka have also in-core documentation for hooks,
even if there's been a lot of reluctance historically about having
them).
--
Michael