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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Feb 14, 2016 at 1:33 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> ... the lock manager lock that protects the fields in a
>>> given PGPROC is chosen by taking pgprocno modulo the number of lock
>>> manager partitions.

>> pgprocno of the specific PGPROC, or of the group leader? If it's
>> the former, I'm pretty suspicious that there are deadlock and/or
>> linked-list-corruption hazards here. If it's the latter, at least
>> the comments around this are misleading.

> Leader. The other way would be nuts.

On closer inspection, that's actually still somewhat problematic,
because that essentially means that no one can inspect another backend's
lockGroupLeader etc fields unless they take *all* the lock partition
LWLocks first. You can't take just the relevant one because you don't
know which one that is, at least not without dereferencing lockGroupLeader
which is entirely unsafe if you haven't got the appropriate lock.

This isn't a problem for members of the lock group, since they already
know who the leader is and hence which partition lock to use. And it's
not a problem for the deadlock detector either since it'll take all those
partition locks anyway. But it makes life difficult for status inspection
code. I'm finding that the only way to write a lock-group-aware function
that tells whether A is blocked by B is to hold both the ProcArrayLock and
*all* of the lock partition locks throughout inspection of the data
structures. I'd hoped to be able to restrict it to locking just the
partition holding the lock A is blocked on, but that ain't working.

Maybe this is all okay, but it makes me nervous that the data structures
may have been over-optimized.

regards, tom lane