Re: race condition in pg_class

Nitin Motiani <nitinmotiani@google.com>

From: Nitin Motiani <nitinmotiani@google.com>
To: Noah Misch <noah@leadboat.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-06T09:52:48Z
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 Fri, Sep 6, 2024 at 3:34 AM Noah Misch <noah@leadboat.com> wrote:
>
> On Thu, Sep 05, 2024 at 07:10:04PM +0530, Nitin Motiani wrote:
> > On Thu, Sep 5, 2024 at 1:27 AM Noah Misch <noah@leadboat.com> wrote:
> > > On Wed, Sep 04, 2024 at 09:00:32PM +0530, Nitin Motiani wrote:
> > > >  Assert(rel->ri_needsLockTagTuple ==  IsInplaceUpdateRelation(rel->relationDesc)
> > > >
> > > > This can safeguard against users of ResultRelInfo missing this field.
> > >
> > > v10 does the rename and adds that assertion.  This question remains open:
> >
> > Looks good. A couple of minor comments :
> > 1. In the inplace110 commit message, there are still references to
> > heap_inplace_update. Should it be clarified that the function has been
> > renamed?
>
> PGXN has only one caller of this function, so I think that wouldn't help
> readers enough.  If someone gets a compiler error about the old name, they'll
> figure it out without commit log guidance.  If a person doesn't get a compiler
> error, they didn't need to read about the fact of the rename.
>
> > 2. Should there be a comment above the ri_needLockTag definition in
> > execNodes.h that we are caching this value to avoid function calls to
> > IsInPlaceUpdateRelation for every tuple? Similar to how the comment
> > above ri_TrigFunctions mentions that it is cached lookup info.
>
> Current comment:
>
>         /* updates do LockTuple() before oldtup read; see README.tuplock */
>         bool            ri_needLockTagTuple;
>
> Once the comment doesn't fit in one line, pgindent rules make it take a
> minimum of four lines.  I don't think words about avoiding function calls
> would add enough value to justify the vertical space, because a person
> starting to remove it would see where it's called.  That's not to say the
> addition would be negligent.  If someone else were writing the patch and had
> included that, I wouldn't be deleting the material.

Thanks. I have no other comments.