Changeset 174009 in webkit

Timestamp:

Sep 26, 2014 9:58:41 AM (10 years ago)

Author:

ap@apple.com

Message:

Get rid of Retry status in webkit-queues
​https://bugs.webkit.org/show_bug.cgi?id=137135

Reviewed by Ryosuke Niwa.

• QueueStatusServer/config/messages.py: Removed Retry.

• QueueStatusServer/handlers/releasepatch.py: This is now straightforward, as it

no longer needs to check the latest status. It just always both unlocks the patch
and removes it from WorkItems.

• QueueStatusServer/handlers/submittoews.py: (SubmitToEWS._should_add_to_ews_queue):

I don't understand why we even needed to check for retries here, but now that there
are no retries, that code can go to /dev/null.

• QueueStatusServer/loggers/recordpatchevent.py:

(RecordPatchEvent.started):
(RecordPatchEvent.retrying): Deleted.
Fixed retry counting, it should work for all queues now.

• QueueStatusServer/model/queuestatus.py:

(QueueStatus.is_retry_request): Deleted. These are no more.

• Scripts/webkitpy/common/net/statusserver_mock.py:

(MockStatusServer.release_lock):

• Scripts/webkitpy/tool/commands/queues_unittest.py:

Did whatever it took to keep passing the tests. The particular test doesn't seem
quite right, but whatever.

• Scripts/webkitpy/tool/commands/queues.py:

(CommitQueue.process_work_item): Instead of posting a retry status, just unlock
and let others pick up. Also, added explicit returns for clarity.
(AbstractPatchQueue._did_retry): Deleted.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• QueueStatusServer/app.yaml (modified) (1 diff)
• QueueStatusServer/config/messages.py (modified) (1 diff)
• QueueStatusServer/handlers/releasepatch.py (modified) (1 diff)
• QueueStatusServer/handlers/submittoews.py (modified) (1 diff)
• QueueStatusServer/loggers/recordpatchevent.py (modified) (3 diffs)
• QueueStatusServer/model/queuestatus.py (modified) (1 diff)
• Scripts/webkitpy/common/net/statusserver_mock.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/queues.py (modified) (4 diffs)
• Scripts/webkitpy/tool/commands/queues_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2014-09-26  Alexey Proskuryakov  <ap@apple.com>
+
+        Get rid of Retry status in webkit-queues
+        https://bugs.webkit.org/show_bug.cgi?id=137135
+
+        Reviewed by Ryosuke Niwa.
+
+        * QueueStatusServer/config/messages.py: Removed Retry.
+
+        * QueueStatusServer/handlers/releasepatch.py: This is now straightforward, as it
+        no longer needs to check the latest status. It just always both unlocks the patch
+        and removes it from WorkItems.
+
+        * QueueStatusServer/handlers/submittoews.py: (SubmitToEWS._should_add_to_ews_queue):
+        I don't understand why we even needed to check for retries here, but now that there
+        are no retries, that code can go to /dev/null.
+
+        * QueueStatusServer/loggers/recordpatchevent.py:
+        (RecordPatchEvent.started):
+        (RecordPatchEvent.retrying): Deleted.
+        Fixed retry counting, it should work for all queues now.
+
+        * QueueStatusServer/model/queuestatus.py:
+        (QueueStatus.is_retry_request): Deleted. These are no more.
+
+        * Scripts/webkitpy/common/net/statusserver_mock.py:
+        (MockStatusServer.release_lock):
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        Did whatever it took to keep passing the tests. The particular test doesn't seem
+        quite right, but whatever.
+
+        * Scripts/webkitpy/tool/commands/queues.py:
+        (CommitQueue.process_work_item): Instead of posting a retry status, just unlock
+        and let others pick up. Also, added explicit returns for clarity.
+        (AbstractPatchQueue._did_retry): Deleted.
+
 2014-09-26  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/Tools/QueueStatusServer/app.yaml

@@ -1 +1 @@
 application: webkit-queues
-version: 173979 # Bugzilla bug ID of last major change
+version: 174009 # Bugzilla bug ID of last major change
 runtime: python
 api_version: 1

trunk/Tools/QueueStatusServer/config/messages.py

@@ -30 +30 @@
 pass_status = "Pass"
 fail_status = "Fail"
-retry_status = "Retry"
 error_status = "Error"

trunk/Tools/QueueStatusServer/handlers/releasepatch.py

@@ -55 +55 @@
         # WorkItems and ActiveWorkItems.
 
-        # Only remove it from the queue if the last message is not a retry request.
-        # Allow removing it from the queue even if there is no last_status for easier testing.
-        if not last_status or not last_status.is_retry_request():
-            queue.work_items().remove_work_item(attachment_id)
-            RecordPatchEvent.stopped(attachment_id, queue_name)
-        else:
-            RecordPatchEvent.retrying(attachment_id, queue_name)
+        queue.work_items().remove_work_item(attachment_id)
+        RecordPatchEvent.stopped(attachment_id, queue_name)
 
-        # Always release the lock on the item.
         queue.active_work_items().expire_item(attachment_id)

trunk/Tools/QueueStatusServer/handlers/submittoews.py

@@ -46 +46 @@
         assert(queue.is_ews())
         latest_status = attachment.status_for_queue(queue)
-        if not latest_status:
-            return True
-        # Only ever re-submit to the EWS if the EWS specifically requested a retry.
-        # This allows us to restart the EWS feeder queue, without all r? patches
-        # being retried as a result of that restart!
-        # In some future version we might add a "force" button to allow the user
-        # to override this restriction.
-        return latest_status.is_retry_request()
+        # The feeder queue only submits each patch once normally, but it loses its memory
+        # when restarted. We do not want to re-add all patches if that happens.
+        # In the future we might add a "force" button to allow the user to retry patches
+        # that were previously submitted.
+        return not latest_status
 
     def _add_attachment_to_ews_queues(self, attachment):

trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py

@@ -1 +1 @@
 # Copyright (C) 2013 Google Inc. All rights reserved.
+# Copyright (C) 2014 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -42 +43 @@
 
     @classmethod
-    def retrying(cls, attachment_id, queue_name, bot_id=None):
-        patch_log = PatchLog.lookup_if_exists(attachment_id, queue_name)
-        if not patch_log:
-            WarningLog.record("patchlog missing", "In retrying event.", attachment_id, queue_name, bot_id)
-            return
-
-        if bot_id:
-            patch_log.bot_id = bot_id
-        patch_log.retry_count += 1
-        patch_log.put()
-
-        queue_log = QueueLog.get_current(queue_name, queue_log_duration)
-        queue_log.patch_retry_count += 1
-        queue_log.put()
-
-    @classmethod
     def started(cls, attachment_id, queue_name, bot_id=None):
         patch_log = PatchLog.lookup_if_exists(attachment_id, queue_name)
@@ -64 +49 @@
             return
 
-        # An existing wait_duration implies the patch had been started previously and is being picked up again because it had expired.
-        if not patch_log.wait_duration:
-            if bot_id:
-                patch_log.bot_id = bot_id
+        if bot_id:
+            patch_log.bot_id = bot_id
+
+        # An existing wait_duration implies the patch had been started previously and is
+        # being picked up again because it had expired, or was released.
+        if patch_log.wait_duration:
+            patch_log.retry_count += 1
+            patch_log.put()
+
+            queue_log = QueueLog.get_current(queue_name, queue_log_duration)
+            queue_log.patch_retry_count += 1
+            queue_log.put()
+        else:
             patch_log.calculate_wait_duration()
             patch_log.put()

trunk/Tools/QueueStatusServer/model/queuestatus.py

@@ -41 +41 @@
     date = db.DateTimeProperty(auto_now_add=True)
     results_file = db.BlobProperty()
-
-    def is_retry_request(self):
-        return self.message == messages.retry_status

trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py

@@ -53 +53 @@
         _log.info("MOCK: release_work_item: %s %s" % (queue_name, patch.id()))
 
+    def release_lock(self, queue_name, patch):
+        _log.info("MOCK: release_lock: %s %s" % (queue_name, patch.id()))
+
     def update_work_items(self, queue_name, work_items):
         self._work_items = work_items

trunk/Tools/Scripts/webkitpy/tool/commands/queues.py

@@ -66 +66 @@
     _pass_status = "Pass"
     _fail_status = "Fail"
-    _retry_status = "Retry"
     _error_status = "Error"
 
@@ -240 +239 @@
         self._release_work_item(patch)
 
-    def _did_retry(self, patch):
-        self._update_status(self._retry_status, patch)
-        self._release_work_item(patch)
-
     def _did_error(self, patch, reason):
         message = "%s: %s" % (self._error_status, reason)
@@ -326 +321 @@
                 self._did_pass(patch)
                 return True
-            self._did_retry(patch)
+            self._unlock_patch(patch)
+            return False
         except ScriptError, e:
             validator = CommitterValidator(self._tool)
@@ -334 +330 @@
                 self._upload_results_archive_for_patch(patch, results_archive)
             self._did_fail(patch)
+            return False
 
     def _failing_tests_message(self, task, patch):

trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py

@@ -406 +406 @@
 Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --port=mac
 MOCK: update_status: commit-queue Passed tests
-MOCK: update_status: commit-queue Retry
-MOCK: release_work_item: commit-queue 10000
+MOCK: release_lock: commit-queue 10000
 """
         self.maxDiff = None