Horton

Re: race condition in pg_class

Noah Misch <noah@leadboat.com>

View thread
From: Noah Misch <noah@leadboat.com>
To: Nitin Motiani <nitinmotiani@google.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>, Smolkin Grigory <smallkeen@gmail.com>, Michael Paquier <michael@paquier.xyz>, Alexander Lakhin <exclusion@gmail.com>
Date: 2024-09-19T21:33:46Z
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 Mon, Sep 09, 2024 at 10:55:32AM +0530, Nitin Motiani wrote:
> On Sat, Sep 7, 2024 at 12:25 AM Noah Misch <noah@leadboat.com> wrote:
> > https://commitfest.postgresql.org/49/5090/ remains in status="Needs review".
> > When someone moves it to status="Ready for Committer", I will commit
> > inplace090, inplace110, and inplace120 patches.  If one of you is comfortable
> > with that, please modify the status.
> 
> Done.

FYI, here are the branch-specific patches.  I plan to push these after the v17
release freeze lifts next week.  Notes from the back-patch:

1. In v13 and v12, "UPDATE pg_class" or "UPDATE pg_database" can still lose a
concurrent inplace update.  The v14+ fix relied on commit 86dc900 "Rework
planning and execution of UPDATE and DELETE", which moved the last fetch of
the pre-update tuple into nodeModifyTable.c.  Fixing that was always optional.
I prefer leaving it unfixed in those two branches, as opposed to writing a fix
specific to those branches.  Here's what I put in v13 and v12:

 		/*
+		 * We lack the infrastructure to follow rules in README.tuplock
+		 * section "Locking to write inplace-updated tables".  Specifically,
+		 * we lack infrastructure to lock tupleid before this file's
+		 * ExecProcNode() call fetches the tuple's old columns.  Just take a
+		 * lock that silences check_lock_if_inplace_updateable_rel().  This
+		 * doesn't actually protect inplace updates like those rules intend,
+		 * so we may lose an inplace update that overlaps a superuser running
+		 * "UPDATE pg_class" or "UPDATE pg_database".
+		 */
+#ifdef USE_ASSERT_CHECKING
+		if (IsInplaceUpdateRelation(resultRelationDesc))
+		{
+			lockedtid = *tupleid;
+			LockTuple(resultRelationDesc, &lockedtid, InplaceUpdateTupleLock);
+		}
+		else
+			ItemPointerSetInvalid(&lockedtid);
+#endif


2. The other area of tricky conflicts was the back-patch in
ExecMergeMatched(), from v17 to v16.

3. I've added inplace088-SetRelationTableSpace, a back-patch of refactoring
commits 4c9c359 and 2484329 to v13 and v12.  Before those commits, we held the
modifiable copy of the relation's pg_class row throughout a
table_relation_copy_data().  Back-patching it avoids a needless long-duration
LOCKTAG_TUPLE, and it's better than implementing a novel way to avoid that.