On Sat, Aug 1, 2015 at 5:00 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Fabrízio de Royes Mello wrote:
>
>> In this patch I didn't change all lockmode comparison places previous
>> pointed by you, but I can change it maybe adding other method called
>> LockModeIsValid(lockmode) to do the comparison "lockmode >= NoLock &&
>> lockmode < MAX_LOCKMODES" used in many places.
>
> I don't like this. Is it possible to write these comparisons in terms
> of what they conflict with? I think there are two main cases in the
> existing code:
>
> 1. "is this lock mode valid" (sounds reasonable)
> 2. "can this be acquired in hot standby" (not so much, but makes
> sense.)
>
> and now we have your third thing, "what is the strongest of these two
> locks".
The third method already exists in tablecmds.c: /* * Take the greatest lockmode from any subcommand
*/ if (cmd_lockmode > lockmode) lockmode = cmd_lockmode;
> For instance, if you told me to choose between ShareLock and
> ShareUpdateExclusiveLock I wouldn't know which one is strongest. I
> don't it's sensible to have the "lock mode compare" primitive honestly.
> I don't have any great ideas to offer ATM sadly.
Yes, the thing is that lowering the lock levels is good for
concurrency, but the non-monotony of the lock levels makes it
impossible to choose an intermediate state correctly. Hence I think
that the one and only safe answer to this problem is that we check if
the locks taken for each subcommand are all the same, and use this
lock. If there is just one lock different, like for example one
subcommand uses ShareLock, and a second one ShareUpdateExclusiveLock
(good example of yours) we simply fallback to AccessExclusiveLock,
based on the fact that we are sure that this lock conflicts with all
the other ones.
At the same time I think that we should as well patch the existing
code path of tablecmds.c that already does those comparisons.Regards,
--
Michael
