Kurt Deschler has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20355 )

Change subject: IMPALA-12364: Display memory, disk and network metrics in 
webUI's query timeline
......................................................................


Patch Set 2:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl
File www/query_timeline.tmpl:

http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@65
PS2, Line 65:     <input type="checkbox" id="plan_order" 
onClick="renderFragmentDiagram()"/>
Need a way to hide the fragment chart too. Maybe clocking on the fragment chart 
to add an X to the corner of the fragment chart to close it.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@469
PS2, Line 469:   var display_height = Math.min(window.innerHeight - 
timing_diagram.offsetTop - 50
Timing height bound should not go below 200:

  var display_height = Math.min(Math.max(200, window.innerHeight - timing_diag
        - (utilization_metrics_parse_successful? getUtilizationHeight() : 0)
        - (fragment_metrics_toogled? getFragmentMetricsHeight() : 0))
        , rownum * row_height);


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@471
PS2, Line 471:       - (fragment_id_selected == undefined? 0 : 
getFragmentMetricsHeight())
Probably better to show fragment metric after periodic metrics so the periodic 
metric don't move. Also would be nice if you can display multiple fragment 
metrics at the same time.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@670
PS2, Line 670:   return window.innerHeight * 0.3;
Change to:
Math.min(window.innerHeight * 0.3, 200);


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@674
PS2, Line 674:   return window.innerHeight * 0.2;
Math.min(window.innerHeight * 0.3, 200);


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@695
PS2, Line 695:   var units = ['B/s', 'KB/s', 'MB/s', 'GB/s', 'TB/s', 'PB/s'];
This should not be B units for Memory, not B/s rates.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@766
PS2, Line 766:   axis_mappings[fragment_metrics_counter_names[0]] = 'y';
These should be mapped by label rather than position.


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@1018
PS2, Line 1018:     clearTimesamples(sampled_utilization_timeseries, 
max_samples_utilization);
Can we aggregate the fragment metrics into the periodic metrics too?


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@1106
PS2, Line 1106:     timestamp_gridline = get_svg_line(stroke_fill_colors.black, 
e.pageX, 0, e.pageX,
can we make this so the cursors on all charts move together?


http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@1151
PS2, Line 1151: window.addEventListener('resize', function(event) {
Zooming out in the browser (WITH ctrl-'-') then back in (ctrl-'+') is not 
currently restoring the original layout.



--
To view, visit http://gerrit.cloudera.org:8080/20355
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f
Gerrit-Change-Number: 20355
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Thu, 17 Aug 2023 18:07:09 +0000
Gerrit-HasComments: Yes

Reply via email to