[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/19417 )
Change subject: IMPALA-11823: Add more items to Impala web UI queries page ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/19417/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19417/2//COMMIT_MSG@10 PS2, Line 10: can visually see > nit: replace with "show" Done http://gerrit.cloudera.org:8080/#/c/19417/2//COMMIT_MSG@13 PS2, Line 13: > nit: remove one space here Done http://gerrit.cloudera.org:8080/#/c/19417/2/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/19417/2/be/src/runtime/coordinator.cc@1429 PS2, Line 1429: UpdatePeakMemUsageAndBytesRead > nit: Maybe we can use the existing > Coordinator::ComputeQueryResourceUtilization > to get the total resource usage of a query. Besides, do you think > the total bytes_sent metric is also worth adding to the web page? Thanks for the advice! I will try to put bytes_sent also to the page, and try to get the total resource usage of a query from ComputeQueryResourceUtilization. http://gerrit.cloudera.org:8080/#/c/19417/2/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/19417/2/be/src/service/client-request-state.h@657 PS2, Line 657: Are > nit: remove "Are" Done http://gerrit.cloudera.org:8080/#/c/19417/2/common/protobuf/admission_control_service.proto File common/protobuf/admission_control_service.proto: http://gerrit.cloudera.org:8080/#/c/19417/2/common/protobuf/admission_control_service.proto@193 PS2, Line 193: microseconds > milliseconds? 1s = 1000 milliseconds. 1 milliseconds = 1000 milliseconds Done http://gerrit.cloudera.org:8080/#/c/19417/2/www/queries.tmpl File www/queries.tmpl: http://gerrit.cloudera.org:8080/#/c/19417/2/www/queries.tmpl@31 PS2, Line 31: <th>Details</th> : <th>Action</th> > Not sure if moving "Details" and "Action" to the front will break > any apps. Is there any motivations for moving? Thanks for the code review! There are some reason, in practice, we find it inconvenient to put "Details" and "Action" at the end of the list, because we often need to drag the browser slider to the right to click the button in the case of a long record. -- To view, visit http://gerrit.cloudera.org:8080/19417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19c75461a6405025fa433ae84d2c94d013fcaacb Gerrit-Change-Number: 19417 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Fri, 03 Feb 2023 12:33:52 +0000 Gerrit-HasComments: Yes
