Bug 137129 – EWS only repeats its cycle every two hours

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 238673 [details]
proposed fix

[WebKit Commit Bot]

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.

[Alexey Proskuryakov]

Created attachment 238674 [details]
proposed fix

Indeed

[Alexey Proskuryakov]

Created attachment 238675 [details]
ok, remembered to run test-webkitpy now

[Ryosuke Niwa]

Comment on attachment 238675 [details]
ok, remembered to run test-webkitpy now

Hm... this is missing _unlock.

[Alexey Proskuryakov]

Created attachment 238681 [details]
proposed fix

How about that?

[WebKit Commit Bot]

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.

[Ryosuke Niwa]

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.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r173979>.