Re: TopoSort() fix

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2019-07-30T17:40:06Z
Lists: pgsql-hackers
On Tue, Jul 30, 2019 at 1:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> No, there's a sufficient reason why we should force advisory locks
> to be taken in the leader process, namely that the behavior is totally
> different if we don't: they will disappear at the end of the parallel
> worker run, not at end of transaction or session as documented.

Oh, good point.  I forgot about that.

> However, that argument doesn't seem to be a reason why the advisory-lock
> functions couldn't be parallel-restricted rather than parallel-unsafe.

Agreed.

> In any case, my question at the moment is whether we need the belt-and-
> suspenders-too approach of having both non-parallel-safe marking and an
> explicit check inside these functions.  We've largely moved away from
> hard-wired checks for e.g. superuserness, and surely these things are
> less dangerous than most formerly-superuser-only functions.

If we can't think of a way that the lack of these checks could crash
it, then I think it's OK to remove the hardwired checks.  If we can,
I'd favor keeping them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Commits

  1. Add an isolation test to exercise parallel-worker deadlock resolution.

  2. Mark advisory-lock functions as parallel restricted, not parallel unsafe.

  3. Fix busted logic for parallel lock grouping in TopoSort().