Kurt Deschler 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' Done http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@199 PS5, Line 199: % > nit: add space around '%' Done 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? Done 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' Done http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@237 PS5, Line 237: > nit: remove empty line Done http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@244 PS5, Line 244: > nit: extra space Done http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@322 PS5, Line 322: * > nit: space around '*' Done http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@341 PS5, Line 341: / > nit: space around '/' in this line Done 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' Done 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 Done 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' Done 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 nee Just looked and unshift is O(n) per call. push is O(1) and reverse is O(n). Going to leave this as-is. http://gerrit.cloudera.org:8080/#/c/19583/5/www/query_timeline.tmpl@451 PS5, Line 451: + > nit: space around '+' Done -- 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 13:25:20 +0000 Gerrit-HasComments: Yes
