[devel at bb.net] exclusive locks & fairness

[Vitali Lovich]

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 across all
botmasters or not.

The problem 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/20151108/c7bfb57f/attachment.html>