<div dir="ltr"><div><div>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 :/<br><br></div>It might be possible to move locks to the database, and use table locking to ensure atomicity?<br><br></div>Dustin<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 4, 2015 at 7:01 PM, Vitali Lovich <span dir="ltr"><<a href="mailto:vlovich@gmail.com" target="_blank">vlovich@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">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.<div>I think there are 2 limitations:</div><div><br></div><div>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.</div><div>2. There should be a way to distinguish greedy from non-greedy access.</div><div><br></div><div>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.</div><div>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).</div><div><br></div><div>I’m not sure where to start fixing #1 - would appreciate any pointers.</div><div>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.</div><div><br></div><div>-Vitali<br><div><br><div><blockquote type="cite"><div>On Nov 4, 2015, at 2:56 PM, Dustin J. Mitchell <<a href="mailto:dustin@v.igoro.us" target="_blank">dustin@v.igoro.us</a>> wrote:</div><br><div><div dir="ltr">I think your analysis is correct.<br><div><br></div><div>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.<br><br></div><div>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.<br><br></div><div>Dustin<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 3, 2015 at 11:38 PM, Vitali Lovich <span dir="ltr"><<a href="mailto:vlovich@gmail.com" target="_blank">vlovich@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
My impression is that there is no fairness for locks currently & that exclusive locks can starve.  Is that true?<br>
<br>
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,<br>
it will continue since it can acquire the lock thus preventing B from running.<br>
<br>
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?<br>
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.<br>
<br>
Thanks,<br>
Vitali<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@buildbot.net" target="_blank">devel@buildbot.net</a><br>
<a href="https://lists.buildbot.net/mailman/listinfo/devel" rel="noreferrer" target="_blank">https://lists.buildbot.net/mailman/listinfo/devel</a></blockquote></div><br></div>
</div></blockquote></div><br></div></div></div></blockquote></div><br></div>