Surya Hebbar 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 18: (12 comments) Now, I have split the test changes into a separate patch as required. http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@8 PS17, Line 8: > Add: Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@9 PS17, Line 9: This patch adds fragment-level metrics to the WebUI query timeline > Limit line length to 73 characters in commit message. Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@10 PS17, Line 10: nd divides the query time > fragment's row in the phase Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@25 PS17, Line 25: - MemoryUsage > Do you mean they will be updated for a running query? Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@35 PS17, Line 35: A warning is displayed on clicking a fragment with less number of samples > Say that RESOURCE_TRACE_RATIO must be set to enable periodic metrics and re Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@48 PS17, Line 48: : > are displayed Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@59 PS17, Line 59: rolling. > What is preventing automated testing? I meant automated tests have been written in order to reduce manual testing. http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@59 PS17, Line 59: horizontal scrolling. > We may want to split the test changes into a separate patch. Done http://gerrit.cloudera.org:8080/#/c/20355/17//COMMIT_MSG@76 PS17, Line 76: > Unclear what this is relevant to. Is this part of the testing infrastructur It was mentioned to address that this comment had been resolved... I'm a bit concern that look up on time_series_counters indices is straight by the index rather than by checking the name first (time_series_counters[i + offset_counters].name). There is no guarantee that the order will be the same all or there will be no new counter that will change the order in the future. Thus, we have name field in TTimeSeriesCounter. Rather than offset_counters, maybe should pass the exact counter name you're targeting (say, HostCpuSysPercentage) and look for counter with matching name. 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@471 PS2, Line 471: > First comment was relating to the position of the charts meaning make the f Done http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@778 PS2, Line 778: > The x-axis for the periodic metrics and the phase diagram have exactly the Done http://gerrit.cloudera.org:8080/#/c/20355/2/www/query_timeline.tmpl@1018 PS2, Line 1018: > Makes sense. So we can only show 2 scales or can there be other sets of lab Yes, only 2 scales/units that match either y1 or y2 axis can be shown. Other labels can be shown by scaling, but they would not relate to either axes. -- 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: 18 Gerrit-Owner: Surya Hebbar <sheb...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Surya Hebbar <sheb...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 05 Oct 2023 17:01:48 +0000 Gerrit-HasComments: Yes
