[devel at bb.net] exclusive locks & fairness

[Dustin J. Mitchell]

That's true, yes.

Dustin

On Wed, Nov 4, 2015 at 7:51 PM, Vitali Lovich <vlovich at gmail.com> wrote:

> I think the locking itself is atomic.  I had to read a few times to
> understand exactly how it was working but it does seem to be correct.
> There’s no obvious race conditions inherent to the approach that I could
> find.  Moving to a DB would be for multi-botmaster scenarios but I haven’t
> thought about those.  I’m not sure if a MasterLock would be expected to
> take across all botmasters or not.
>
> The problem I’m looking at is purely fairness & acquisition logic which
> unless the DB implements for us I think we still get back to the same
> problem.
>
> On Nov 4, 2015, at 4:10 PM, Dustin J. Mitchell <dustin at v.igoro.us> wrote:
>
> To be honest, I'm not especially happy with the lock implementation, and I
> don't think anyone really understands it anymore -- you're probably the
> foremost expert, having just read the code :/
>
> It might be possible to move locks to the database, and use table locking
> to ensure atomicity?
>
> Dustin
>
> On Wed, Nov 4, 2015 at 7:01 PM, Vitali Lovich <vlovich at gmail.com> wrote:
>
>> Hmm… Looking at isAvailable it does actually appear to be fair so it
>> won’t acquire a counting lock if there’s an exclusive lock ahead of it.
>> I think there are 2 limitations:
>>
>> 1. Builds are started & block on locks that they can’t acquire.  Any they
>> can are acquired.  The lock acquisition should be acquire all locks or none
>> at all to avoid deadlocks since the lock acquisition order is not enforced
>> at all.  Dining philosopher’s problem.
>> 2. There should be a way to distinguish greedy from non-greedy access.
>>
>> Fixing #1 would ensure it’s almost impossible to set up a lock
>> configuration that deadlocks with no diagnostics possible.  It would also
>> ensure that time estimates are more accurate since waiting for locks
>> wouldn’t count as “build active” time as it does now.
>> Fixing #2 would ensure that it’s possible to have lock importance so that
>> important tasks grabbing a counting lock could postpone less important
>> exclusive lock tasks (e.g. processing some data in response to a user
>> request vs running a background task that modifies the environment).
>>
>> I’m not sure where to start fixing #1 - would appreciate any pointers.
>> I can tackle #2 if that sounds interesting.  I’m thinking something like
>> “preemptable_exclusive” as an additional access that returns false from
>> isAvailable if there’s any counting jobs enqueued.
>>
>> -Vitali
>>
>> On Nov 4, 2015, at 2:56 PM, Dustin J. Mitchell <dustin at v.igoro.us> wrote:
>>
>> I think your analysis is correct.
>>
>> Locks are entirely implemented within a master, which has a good side and
>> a bad side.  On the good side, it means all the data you need is there,
>> somewhere in memory.  On the bad side, it means they don't work too well on
>> a multi-master setup.
>>
>> I think it would make sense to keep list of lock requests, in order.  In
>> other words, a new counting lock can't acquire a lock that's already
>> acquired in counting mode iff there is a pending exclusive lock.
>>
>> Dustin
>>
>> On Tue, Nov 3, 2015 at 11:38 PM, Vitali Lovich <vlovich at gmail.com> wrote:
>>
>>> Hi,
>>>
>>> My impression is that there is no fairness for locks currently & that
>>> exclusive locks can starve.  Is that true?
>>>
>>> In other words, if I have job A that acquires a counting lock & job B
>>> acquires an exclusive lock, job B will correctly wait until A is done.
>>> However, if job C comes in & it in turn also acquires a counting lock,
>>> it will continue since it can acquire the lock thus preventing B from
>>> running.
>>>
>>> Assuming my analysis is correct, is anyone aware of an easy way to fix
>>> this (e.g. via some hook point within buildbot) or where I would even start
>>> to look for it?
>>> For example, is there a way to test if anything is waiting to acquire a
>>> given exclusive lock?  I had a test that just tried to see if it could get
>>> the exclusive lock but I just realized that’s broken since it basically
>>> just transforms that lock into an exclusive lock.
>>>
>>> Thanks,
>>> Vitali
>>> _______________________________________________
>>> devel mailing list
>>> devel at buildbot.net
>>> https://lists.buildbot.net/mailman/listinfo/devel
>>
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/devel/attachments/20151105/b473519c/attachment.html>