EWS never removes invalid patch ids
Created attachment 71624 [details] Patch
Comment on attachment 71624 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71624&action=review I've lost track of how this is supposed to work at this point. > WebKitTools/Scripts/webkitpy/tool/commands/queues.py:214 > + # FIXME: Using a fake patch because release_work_item has the wrong API. > + # We also don't really need to release the lock (although that's fine), > + # mostly we just need to remove this bogus patch from our queue. > + # If for some reason bugzilla is just down, then it will be re-fed later. > + patch = Attachment({'id': patch_id}, None) :(
Understandable. The old EWS system: - Each EWS bot woke up every 2 minutes and grabbed the list of r? patches. - EWS would grab from queues.webkit.org the last status for any patch id (and cache it) - The first patch w/o status would be processed. Pros: - Simple, fully federated. Cons: - Only processes patches which are r? at the time the EWS gets to them. - Only one EWS bot per queue is possible. - WorkItems queue list displayed at queues.webkit.org/queue-status/queue-name is never up-to-date. New EWS System: - A feeder queue wakes up every 30 seconds, looking for r? patches. - Feeder feeds patches into queues.webkit.org via /submit-to-ews - /submit-to-ews checks the last per-queue QueueStatus for the submitted patch_id. If there is no status (or the status is "retry") then the patch_id is added to the WorkItems for that queue. - queues.webkit.org/next-patch/queue-name returns the first patch in the WorkItems list which is not locked by the ActiveWorkItems list. - The individual EWS queues process the patches and are expected to use /release-patch when the patch should be removed from the WorkItems list. Pros: - Any patch which is ever r? for 30s will be processed. - Arbitrary patches can be processes (they don't even need r?) - Any number of bots can service a single queue. - queues.webkit.org/queue-status/queue-name is kept up-to-date by the feeder queue. Cons: - Making all the right code call /release-patch has proved complicated. - We now see (and process) all r? patches (which currently seems more than some bots can handle). - Current design conflates "remove from WorkItems" and "unlock the patch" which it shouldn't. (trunk can't build should "unlock" the patch, but not remove it, etc.) Hopefully that brings you more up-to-speed. This patch just plugs one more /release-patch hole. Once we have the EWS queues back workign reliably, I want to go back and split /release-patch into two separate calls (possibly moving the auto-lock release back into the server -- remember the magic retry status?). I also want to restructure the /release-patch calls so that we only have to make them in one or two places, instead of the 8 that we do now. I'd like to just keep putting /release-patch bandaids on for now until we get the EWS bots back under control. After they're under control then contemplate a design simplification.
Comment on attachment 71624 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71624&action=review >> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:214 >> + # FIXME: Using a fake patch because release_work_item has the wrong API. >> + # We also don't really need to release the lock (although that's fine), >> + # mostly we just need to remove this bogus patch from our queue. >> + # If for some reason bugzilla is just down, then it will be re-fed later. >> + patch = Attachment({'id': patch_id}, None) > > :( I still think this fake attachment is sadness. Will you buy me a pony if this code is still here in a month?
Comment on attachment 71624 [details] Patch Clearing flags on attachment: 71624 Committed r70409: <http://trac.webkit.org/changeset/70409>
All reviewed patches have been landed. Closing bug.