Re: alternative model for handling locking in parallel groups - Mailing list pgsql-hackers

Hi,

On 2014-11-13 15:59:11 -0500, Robert Haas wrote:
> Discussion of my incomplete group locking patch seems to have
> converged around two points: (1) Everybody agrees that undetected
> deadlocks are unacceptable.  (2) Nobody agrees with my proposal to
> treat locks held by group members as mutually non-conflicting.  As was
> probably evident from the emails on the other thread, it was not
> initially clear to me why you'd EVER want heavyweight locks held by
> different group members to mutually conflict, but after thinking it
> over for a while, I started to think of cases where you would
> definitely want that:

> 1. Suppose two or more parallel workers are doing a parallel COPY.
> Each time the relation needs to be extended, one backend or the other
> will need to take the relation extension lock in Exclusive mode.
> Clearly, taking this lock needs to exclude both workers in the same
> group and also unrelated processes.
> 
> 2. Suppose two or more parallel workers are doing a parallel
> sequential scan, with a filter condition of myfunc(a.somecol), and
> that myfunc(a.somecal) updates a tuple in some other table.  Access to
> update that tuple will be serialized using tuple locks, and it's no
> safer for two parallel workers to do this at the same time than it
> would be for two unrelated processes to do it at the same time.
> 
> On the other hand, I think there are also some cases where you pretty
> clearly DO want the locks among group members to be mutually
> non-conflicting, such as:
> 
> 3. Parallel VACUUM.  VACUUM takes ShareUpdateExclusiveLock, so that
> only one process can be vacuuming a relation at the same time.  Now,
> if you've got several processes in a group that are collaborating to
> vacuum that relation, they clearly need to avoid excluding each other,
> but they still need to exclude other people.  And in particular,
> nobody else should get to start vacuuming that relation until the last
> member of the group exits.  So what you want is a
> ShareUpdateExclusiveLock that is, in effect, shared across the whole
> group, so that it's only released when the last process exits.

Note that you'd definitely not want to do this naively - currently
there's baked in assumptions into the vaccum code that only one backend
is doing parts of it.

I think there's

> 4. Parallel query on a locked relation.  Parallel query should work on
> a table created in the current transaction, or one explicitly locked
> by user action.  It's not acceptable for that to just randomly
> deadlock, and skipping parallelism altogether, while it'd probably be
> acceptable for a first version, is not going a good long-term
> solution.

FWIW, I think it's perfectly acceptable to refuse to work in parallel in
that scenario. And not just for now.

The biggest argument I can see to that is parallel index creation.

> It also sounds buggy and fragile for the query planner to
> try to guess whether the lock requests in the parallel workers will
> succeed or fail when issued.

I don't know. Checking whether we hold a self exclusive lock on that
relation doesn't sound very problematic to me.

> Figuring such details out is the job of
> the lock manager or the parallelism infrastructure, not the query
> planner.

I don't think you can sensibly separate the parallelism infrastructure
and the query planner.

> After thinking about these cases for a bit, I came up with a new
> possible approach to this problem.  Suppose that, at the beginning of
> parallelism, when we decide to start up workers, we grant all of the
> locks already held by the master to each worker (ignoring the normal
> rules for lock conflicts).  Thereafter, we do everything the same as
> now, with no changes to the deadlock detector.  That allows the lock
> conflicts to happen normally in the first two cases above, while
> preventing the unwanted lock conflicts in the second two cases.

I don't think that's safe enough. There's e.g. a reason why ANALYZE
requires SUE lock. It'd definitely not be safe to simply grant the
worker a SUE lock, just because the master backend already analyzed it
or something like that. You could end up with the master and worker
backends ANALYZEing the same relation.

That said, I can definitely see use for a infrastructure where we
explicitly can grant another backend a lock that'd conflict with one
we're already holding. But I think it'll need to be more explicit than
just granting all currently held locks at the "highest" current
level. And I think it's not necessarily going to be granting them all
the locks at their current levels.

What I'm thinking of is basically add a step to execMain.c:InitPlan()
that goes through the locks and grants the child process all the locks
that are required for the statement to run. But not possibly preexisting
higher locks.

Greetings,

Andres Freund