Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20008 )
Change subject: IMPALA-12182: Add CPU utilization chart for RuntimeProfile's sampled metrics ...................................................................... Patch Set 1: (8 comments) Tried to apply your patch on my local environment with Ubuntu 18.04. But the CPU utilization chart was not shown on the query timeline web page so I cannot verify if the implementation is correct. http://gerrit.cloudera.org:8080/#/c/20008/1/.gitattributes File .gitattributes: http://gerrit.cloudera.org:8080/#/c/20008/1/.gitattributes@31 PS1, Line 31: www/chartist/chartist.umd.min.js binary : www/chartist/chartist.min.css binary Insert these two lines in the right position to keep alphabet order http://gerrit.cloudera.org:8080/#/c/20008/1/LICENSE.txt File LICENSE.txt: http://gerrit.cloudera.org:8080/#/c/20008/1/LICENSE.txt@1123 PS1, Line 1123: Permission is hereby granted, free of charge, to any person obtaining a copy of this software and a Please wrap up the long lines and keep indents as other licenses. http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl File www/query_timeline.tmpl: http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@589 PS1, Line 589: /* approximate timestamps based on sample collections from periodic counter updater : var cur_sample_len = cpu_nodes_usage_aggregate[0].length; : cpu_timeticks = new Array(cur_sample_len); : cpu_timeticks[0] = 0; : for (var i = 1; i < cur_sample_len ; ++i) { : cpu_timeticks[i] = cur_sample_base_time * (i + 1) / (1000 * cur_sample_len); : } : cpu_utilization_chart.data.labels = cpu_timeticks;*/ remove if not useful http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@640 PS1, Line 640: else if (index % 3 == 0) { : return value.toFixed(2); : } : else { Keep code style consistent as: } else if ... { ... } else { ... } http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@661 PS1, Line 661: if ( wrap up long line and remove extra space around "(" and "0" http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@667 PS1, Line 667: utilization_metrics_available = false; move this line in front of line 666 to keep consistent http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@681 PS1, Line 681: key === "End Time").value == "") { add two more indent spaces http://gerrit.cloudera.org:8080/#/c/20008/1/www/query_timeline.tmpl@689 PS1, Line 689: collectUtilizationFromProfile(); : renderUtilization(); collectUtilizationFromProfile() set utilization_metics_parse_successful as true or false. Should we call renderUtilization() if utilization_metics_parse_successful is set as false? -- To view, visit http://gerrit.cloudera.org:8080/20008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idea2a6db217dbfaa7a0695aeabb6d9c1ecf62158 Gerrit-Change-Number: 20008 Gerrit-PatchSet: 1 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: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Tue, 06 Jun 2023 20:16:59 +0000 Gerrit-HasComments: Yes
