Bug 137649

Summary: Add more detailed wait time information to EWS metrics
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, lforschler, rniwa, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch dbates: review+

Description Alexey Proskuryakov 2014-10-12 12:05:03 PDT
Median time is good, but we also need to know whether EWS pipes get clogged during peak hours.
Comment 1 Alexey Proskuryakov 2014-10-12 12:11:23 PDT
Created attachment 239706 [details]
proposed patch
Comment 2 Daniel Bates 2014-10-12 23:34:41 PDT
Comment on attachment 239706 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239706&action=review

> Tools/ChangeLog:11
> +        There is always some wait due to polling nature of the queues, which is well understood

Nit: "due to" => "due to the"

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsAnalyzer.js:396
> +        var patchesThatWaitedMoreThan3Minutes = [];

This is OK as-is. I take it that you find patchesThatWaitedMoreThan3Minutes reads better than patchesThatWaitedMoreThanThreeMinutes.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsBubbleView.js:170
> +            addLine(this.element, formatPercentage(this._results.patchesThatWaitedMoreThan3MinutesCount / this._results.totalPatches) + " of patches had wait time more than 3 minutes.");

Nit: "...had wait time more..." => "...had a wait time of more..."
Comment 3 Daniel Bates 2014-10-12 23:38:22 PDT
Comment on attachment 239706 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239706&action=review

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsAnalyzer.js:396
>> +        var patchesThatWaitedMoreThan3Minutes = [];
> 
> This is OK as-is. I take it that you find patchesThatWaitedMoreThan3Minutes reads better than patchesThatWaitedMoreThanThreeMinutes.

I'm assuming that you plan to make use of this array of patch IDs in a follow up patch. Otherwise, we should make this variable a counter such that its data type is number.
Comment 4 Alexey Proskuryakov 2014-10-13 11:48:42 PDT
Committed <http://trac.webkit.org/r174655>.

> I'm assuming that you plan to make use of this array of patch IDs in a follow up patch. Otherwise, we should make this variable a counter such that its data type is number.

This is a debugging helper that I'd like to keep. When some results look surprising, it helps to have a relatively easy way to tell which patches they were based on.

If the tool takes up, we can have the data in UI one day, to help with addressing issues on the bots.