Re: alternative model for handling locking in parallel groups

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: alternative model for handling locking in parallel groups
Date: 2014-11-14 17:03:05
Message-ID: 20141114170305.GG11733@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2014-11-14 17:12:52 Re: tracking commit timestamps
Previous Message Stephen Frost 2014-11-14 16:47:49 Re: Re: Segmentation fault in pg_dumpall from master down to 9.1 and other bug introduced by RLS