Re: pgsql: Introduce group locking to prevent parallel processes from deadl

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <rhaas(at)postgresql(dot)org>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Introduce group locking to prevent parallel processes from deadl
Date: 2016-02-13 18:32:01
Message-ID: 27041.1455388321@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Robert Haas <rhaas(at)postgresql(dot)org> writes:
> Introduce group locking to prevent parallel processes from deadlocking.

I'm fairly unhappy about this patch, because it has introduced a large
amount of new complexity into the lock manager with effectively no
documentation. Yeah, there's several paragraphs added to lmgr/README,
but they're just a handwavy apologia for the high-level decision to allow
exclusive locks taken by parallel workers to not conflict. I see nothing
whatever explaining how it works, that is any useful docs about the new
data structures or invariants. For example, I can't figure out whether
you have broken the pg_locks display (by causing relevant lock state to
not get displayed, or get displayed in a useless fashion because one
can no longer tell which entries block which other entries). I can't
even tell for sure if it works at all: the README says only that locks
taken by members of the same parallel group don't conflict, but the
implementation looks more like some operations get applied to the group
leader rather than to followers, which is something completely different.

I got here because I thought it would be interesting to see if we could
improve on isolationtester's is-it-waiting query (as suggested by Andres)
by creating a backend function to return all the PIDs that a given PID is
waiting for. I thought this would be reasonably simple to do by crawling
the waitQueue for the lock it's blocked on. However, I can't figure out
what impact the group locking stuff has on that. Maybe I'm just too
stupid, but I prefer to believe this means the code is inadequately
documented.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-02-13 20:42:40 pgsql: Make GetLockStatusData's header comment resemble reality.
Previous Message Tom Lane 2016-02-13 15:16:14 Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.

Browse pgsql-hackers by date

  From Date Subject
Next Message Yury Zhuravlev 2016-02-13 19:28:19 Re: Crash with old Windows on new CPU
Previous Message Andres Freund 2016-02-13 16:31:08 Re: Defaults for replication/backup