Bug 137129

Summary: EWS only repeats its cycle every two hours
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, glenn, rniwa, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed fix
none
proposed fix
none
ok, remembered to run test-webkitpy now
rniwa: review-
proposed fix rniwa: review+

Description Alexey Proskuryakov 2014-09-25 14:42:32 PDT
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.
Comment 1 Alexey Proskuryakov 2014-09-25 15:09:22 PDT
Created attachment 238673 [details]
proposed fix
Comment 2 WebKit Commit Bot 2014-09-25 15:11:39 PDT
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.
Comment 3 Alexey Proskuryakov 2014-09-25 15:14:00 PDT
Created attachment 238674 [details]
proposed fix

Indeed
Comment 4 Alexey Proskuryakov 2014-09-25 15:15:20 PDT
Created attachment 238675 [details]
ok, remembered to run test-webkitpy now
Comment 5 Ryosuke Niwa 2014-09-25 15:36:04 PDT
Comment on attachment 238675 [details]
ok, remembered to run test-webkitpy now

Hm... this is missing _unlock.
Comment 6 Alexey Proskuryakov 2014-09-25 15:58:57 PDT
Created attachment 238681 [details]
proposed fix

How about that?
Comment 7 WebKit Commit Bot 2014-09-25 16:02:20 PDT
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 8 Ryosuke Niwa 2014-09-25 16:07:04 PDT
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.
Comment 9 Alexey Proskuryakov 2014-09-25 16:17:15 PDT
Committed <http://trac.webkit.org/r173979>.