[webkit-reviews] review granted: [Bug 61847] Worker may not be stopped after LT. : [Attachment 95845] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 61847: Worker may not be stopped after LT.
https://bugs.webkit.org/show_bug.cgi?id=61847

Attachment 95845: Patch
https://bugs.webkit.org/attachment.cgi?id=95845&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95845&action=review

>>> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:681
>>> +			     raise AssertionError('Worker %d did not exit in
time' % worker_state.number)
>> 
>> Should we raise an Assertion here or just log the error and try to keep
going?
> 
> Good question. I'm not actually sure what the end result of the run will be
in either case (hung process or process that eventually exits when the slow
thread/process exits). I would prefer to leave it as an assertion for now just
to help identify if it actually occurs.

I think this is OK, but In general I think we should try to have the bots keep
going as much as possible.  If the script crashes, it causes problems for the
gardener who then needs to rollback and wait for the bots to cycle an extra
time.  The downside for you is much smaller: you just need to land the patch
manually and watch the bots as they cycle to see if an error is logged.

You might want to try landing this during non-busy hours or coordinate the
landing with the gardener.