When a patch fails in an uncertain way, so that EWS cannot decide whether it introduces a failure, the whole cycle starts over. How it currently works is that the lock is never released, so the retry happens when the lock expires. That's silly, especially now that the expiry time is two hours - any patch that was submitted to EWS while build was broken will not be retried until two hours later, which is probably longer than it would take to fix the breakage on average. I'm going to make queues release the lock any time they are done.
Created attachment 238673 [details] proposed fix
Attachment 238673 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/Webkit/Tools/Scripts/webkit-patch", line 44, in <module> from webkitpy.tool.main import WebKitPatch File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/main.py", line 41, in <module> from webkitpy.tool import commands File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/__init__.py", line 9, in <module> from webkitpy.tool.commands.earlywarningsystem import AbstractEarlyWarningSystem File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py", line 42, in <module> from webkitpy.tool.commands.queues import AbstractReviewQueue File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/queues.py", line 428 passed = review_patch(patch): ^ SyntaxError: invalid syntax If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 238674 [details] proposed fix Indeed
Created attachment 238675 [details] ok, remembered to run test-webkitpy now
Comment on attachment 238675 [details] ok, remembered to run test-webkitpy now Hm... this is missing _unlock.
Created attachment 238681 [details] proposed fix How about that?
Attachment 238681 [details] did not pass style-queue: ERROR: Tools/QueueStatusServer/model/workitems.py:89: [WorkItems.move_to_end] Instance of 'WorkItems' has no 'key' member [pylint/E1101] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:5: trailing whitespace [pep8/W291] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:6: trailing whitespace [pep8/W291] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:8: trailing whitespace [pep8/W291] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:12: trailing whitespace [pep8/W291] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:34: whitespace after '{' [pep8/E201] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:51: blank line at end of file [pep8/W391] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:51: trailing whitespace [pep8/W291] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:34: [ReleaseLock.get] Instance of 'ReleaseLock' has no 'response' member [pylint/E1101] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:37: [ReleaseLock.post] Instance of 'ReleaseLock' has no 'request' member [pylint/E1101] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:41: [ReleaseLock.post] Instance of 'ReleaseLock' has no 'error' member [pylint/E1101] [5] ERROR: Tools/QueueStatusServer/handlers/releaselock.py:44: [ReleaseLock.post] Instance of 'ReleaseLock' has no '_int_from_request' member [pylint/E1101] [5] Total errors found: 12 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238681 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=238681&action=review > Tools/QueueStatusServer/model/activeworkitems.py:85 > - two_hours_ago = time.mktime((now - timedelta(minutes=120)).timetuple()) > + two_hours_ago = time.mktime((now - work_item_lock_timeout).timetuple()) We should rename two_hours_ago to something like cutoff_time now that the timeout is abstracted by a variable.
Committed <http://trac.webkit.org/r173979>.