Re: group locking: incomplete patch, just for discussion

On 2014-10-31 15:56:42 -0400, Robert Haas wrote:
> On Fri, Oct 31, 2014 at 3:32 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> >> > So just refusing parallelism as soon as anything has taken an access
> >> > exclusive lock doesn't sound too bad to me.
> >>
> >> That restriction seems onerous to me; for example, it would prevent a
> >> parallel sort for CLUSTER or a parallel index rebuild for VACUUM FULL.
> >> Those seems like worthy targets for parallelism, and maybe even early
> >> targets, since I expect a lot of the real complexity here to be in the
> >> query planner, and you can avoid most of that complexity when planning
> >> DDL.
> >
> > Ugh. I think that's aiming too high. You'll suddenly need to share cche
> > invalidations between the backends. I think we should start by requiring
> > that there's no DDL done *at all* in the transaction that starts the
> > parallel activity. For CREATE INDEX PARALLEL that can be done by reusing
> > the logic we have for CONCURRENTLY, thereby moving the parallelized part
> > into a transaction that hasn't done any DDL yet.
> 
> I don't think that's correct.  We only need to process local
> invalidation messages after CommandCounterIncrement(), which I
> anticipate prohibiting during parallel execution (short thought should
> convince you that anything else is completely nuts).

It is more complex, even without CCI. As long as you're doing anything
inside a transaction that already has done DDL, you'll have to play
nasty tricks. Imagine the primary backend has done a BEGIN; CLUSTER
pg_class; and then then starts a child backend. Which will get the same
snapshot, combocids, yada yada. But it *also* will have preexisting
cache entries. Those you need to invalidate before it can do anything
correct.


> > I'm sorry to be a bit pessimistic here. But my intuition says that
> > starting to do group locking opens a can of worms that'll take us a long
> > time to close again. Maybe I'm just imagining complexity that won't
> > actually be there. But I have a hard time believing that.
> 
> What's the distinction between "teach the deadlock detector to catch
> these kind of cases" and "group locking"?  Because I think those are
> at least 90% the same thing.

I understand under 'group locking' that a primary/second backends can
coown a lock that normally is self-exclusive. That's a fair bit more
than adding an edge to the deadlock graph between primary/secondary
backends to make the deadlock detector recognize problems.

What I have serious doubts about is 'coowning' locks. Especially if two
backends normally wouldn't be able to both get that lock.

> > I wonder if we couldn't implement a 'halfway' by allowing parallel
> > workers to jump the lockqueue if the parent process already has the
> > lock. That'd only work for nonconflicting locks, but I think that's
> > actually good.
> 
> The patch takes precisely that approach; that part of the logic is
> already implemented.

Well, my point is that if the solution is just to jump the queue, there
really isn't any data structure changes needed. Secondary acquirers just
need to check whether a lock is already owned by the primary and then
acquire the lock in the absolutely normal way - with a completely
separate entry. Only that they ignored the queue.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services