Jiawei Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 )
Change subject: IMPALA-5149: Provide query profile in JSON format ...................................................................... Patch Set 14: (14 comments) > Uploaded patch set 14. This has been a long process, thanks for your patience on this! http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/service/impala-server.cc@848 PS13, Line 848: Of > offset: $1 Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/service/impala-server.cc@2026 PS13, Line 2026: json_profile(rapidj > json_profile. No need for "_rapid". Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile-test.cc@1166 PS13, Line 1166: RuntimeProfile* profile_ab = RuntimeProfile::Create(&pool, "ProfileAb"); > The name of the profile should probably match the variable name. Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile-test.cc@1204 PS13, Line 1204: for (auto& itr : content["counters"].GetArray()) { : // check normal Counter > Use iterator style like the following. Same at other places. Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile-test.cc@1218 PS13, Line 1218: } > Do we want to check there are no other counter types in content ? Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.h@622 PS13, Line 622: A map of counters name to counter > A map of counter's name to counter Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.h@625 PS13, Line 625: ame, const Counte > This needs to be documented too. Done http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.cc@704 PS13, Line 704: e information > What does all the other information mean ? Done http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@571 PS13, Line 571: download_link = "query_profile_plain_text?query_id={0}".format(query_id) : assert down > nit: one line Done http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@587 PS13, Line 587: > nit: indent 4 for line continuation Done http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@594 PS13, Line 594: le:{0}". > Downloaded ? Done http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@595 PS13, Line 595: the query id > Json pofile Done http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@597 PS13, Line 597: > assert query_id in json_res["contents"]["profile_name"], json_res to help d Done http://gerrit.cloudera.org:8080/#/c/13801/13/www/query_profile.tmpl File www/query_profile.tmpl: http://gerrit.cloudera.org:8080/#/c/13801/13/www/query_profile.tmpl@28 PS13, Line 28: (Available Form > (available formats): Done -- To view, visit http://gerrit.cloudera.org:8080/13801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 14 Gerrit-Owner: Jiawei Wang <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jiawei Wang <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Comment-Date: Tue, 06 Aug 2019 20:12:47 +0000 Gerrit-HasComments: Yes
