Horton

Re: race condition in pg_class

Noah Misch <noah@leadboat.com>

View thread
From: Noah Misch <noah@leadboat.com>
To: Michael Paquier <michael@paquier.xyz>
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-16T14:07:08Z
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.

Attachments

On Sun, Jun 16, 2024 at 09:28:05AM +0900, Michael Paquier wrote:
> 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.

It would be odd to detect exactly 0x0B000000U and not other invalid inputs,
like 0x0A000001U where only 0x0B000001U is valid.  I'm attaching roughly what
it would take.  Shall I squash this into inplace031?

The thing I feel strongly about here is keeping focus on fixing $SUBJECT bugs
that are actually corrupting data out there.  I think we should all limit our
interest in the verbiage of strings that appear only when running developer
tests, especially when $SUBJECT is a bug fix.  When the string appears only
after C code passes invalid input to other C code, it matters even less.

> > 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).

[getting deeply off topic -- let's move this to another thread if it needs to
expand] I like reducing the need to guess.  So far in this inplace update
project (this thread plus postgr.es/m/20240615223718.42.nmisch@google.com),
three patches just fix comments.  Even comments carry quite a price, but I
value them.  When we hand-maintain documentation of a C function in both its
header comment and another place, I get skeptical about whether hackers
(including myself) will actually keep them in sync and skeptical of the
incremental value of maintaining the second version.