Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19583 )

Change subject: IMPALA-11970: Add query timeline display to Impala WebUI
......................................................................


Patch Set 5:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl
File www/query_timeline.tmpl:

http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@185
PS5, Line 185:         for(var instance = 1; instance < 
fp.child_profiles.length; ++instance) {
nit: space after 'for'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@199
PS5, Line 199: %
nit: add space around '%'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@214
PS5, Line 214:           }
Do we need to set node_name for other kinds of scan node?


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@230
PS5, Line 230:           } else if(all_nodes.length) {
nit: space after 'if'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@237
PS5, Line 237:
nit: remove empty line


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@244
PS5, Line 244:
nit: extra space


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@322
PS5, Line 322: *
nit: space around '*'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@341
PS5, Line 341: /
nit: space around '/' in this line


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@392
PS5, Line 392:         if(node.parent_node != undefined) {
nit: space after 'if'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@415
PS5, Line 415: /
nit: space around '/' and '+' i in this line and next three lines


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@429
PS5, Line 429:         if(node.num_children) // Scan (leaf) node
nit: space after 'if'


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@438
PS5, Line 438: pending_fragments.push
Could we use unshift() to add element to the beginning so that we don't need to 
call reverse()?


http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@451
PS5, Line 451: +
nit: space around '+'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b5826107af0f5a7fe306cb986a875cff261d9db
Gerrit-Change-Number: 19583
Gerrit-PatchSet: 5
Gerrit-Owner: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: David Rorke <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 31 Mar 2023 08:12:39 +0000
Gerrit-HasComments: Yes

Reply via email to