[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 15: Verified+1 -- 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: 15 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 07 Aug 2019 21:10:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Reviewed-on: http://gerrit.cloudera.org:8080/13801 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 646 insertions(+), 60 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 16 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 15: Code-Review+2 -- 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: 15 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 07 Aug 2019 17:01:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4754/ DRY_RUN=false -- 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: 15 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 07 Aug 2019 17:01:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Michael Ho 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: Code-Review+2 -- 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 Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 07 Aug 2019 17:01:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4162/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 06 Aug 2019 20:53:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
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(, "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 Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 06 Aug 2019 20:12:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 646 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/14 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 14 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 13: (14 comments) LGTM. Some more minor nits. The patch can be +2'ed after they are addressed. 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: $1 offset: $1 http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/service/impala-server.cc@2026 PS13, Line 2026: json_profile_rapid( json_profile. No need for "_rapid". 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_a2 = RuntimeProfile::Create(, "ProfileAb"); The name of the profile should probably match the variable name. http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile-test.cc@1204 PS13, Line 1204: for (rapidjson::Value::ConstValueIterator itr = content["counters"].Begin(); : itr != content["counters"].End(); ++itr) { Use iterator style like the following. Same at other places. for (auto& itr : content) { 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 ? if (is normal counter) { elif (is high water mark counter) { } else { DCHECK(false) ? } or DCHECK(counter is either normal counter || counters is high watermark counter) 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: get the counter detailed information based on its name A map of counter's name to counter http://gerrit.cloudera.org:8080/#/c/13801/13/be/src/util/runtime-profile.h@625 PS13, Line 625: child_counter_map This needs to be documented too. 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: the other information What does all the other information mean ? 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) nit: one line http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@587 PS13, Line 587: self.ROOT_URL + download_link, query, self.IMPALAD_TEST_PORT) nit: indent 4 for line continuation http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@594 PS13, Line 594: Download Downloaded ? http://gerrit.cloudera.org:8080/#/c/13801/13/tests/webserver/test_web_pages.py@595 PS13, Line 595: Response text Json pofile 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"] assert query_id in json_res["contents"]["profile_name"], json_res to help debugging in case the assert fires. 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: (Choose Format) (available formats): -- 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: 13 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 06 Aug 2019 01:32:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4146/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 13 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 06 Aug 2019 00:26:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 649 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/13 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 13 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 12: (9 comments) We don't really enforce any compatibility for our runtime profile in general now so counters may get added and deleted across releases. So, the json profile output may change as Impala code changes. In the future, when we have a better compatibility story for our profile, it may make sense to have a version string in the output. http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/service/impala-server.cc@845 PS12, Line 845: if (!parse_ok){ The header comment says the output stream is not modified on error. May be worth double checking that json_output not modified on parse error. http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.h@618 PS12, Line 618: void ToJsonCounters(rapidjson::Value* parent, rapidjson::Document* d, : const string& counter_name, const CounterMap& counter_map, : const ChildCounterMap& child_counter_map) const; Please document each of the parameters. http://gerrit.cloudera.org:8080/#/c/13801/11/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/13801/11/be/src/util/runtime-profile.h@126 PS11, Line 126: Get the actual Counter derived type Returns the name of the counter type. http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.cc@766 PS12, Line 766: _rapid I noticed that a lot of the Value variables have the "_rapid" suffix. Is "_json" a more appropriate suffix ? http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.cc@784 PS12, Line 784: info_strings_.find(key)->second. DCHECK(info_strings_.find(key) != info_strings_.end()); http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.cc@802 PS12, Line 802: nit: unnecessary blank line http://gerrit.cloudera.org:8080/#/c/13801/12/be/src/util/runtime-profile.cc@869 PS12, Line 869: nit: unnecessary blank line http://gerrit.cloudera.org:8080/#/c/13801/12/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/12/tests/webserver/test_web_pages.py@563 PS12, Line 563: ("Text", "Json") ["Text", "Json"] http://gerrit.cloudera.org:8080/#/c/13801/12/tests/webserver/test_web_pages.py@563 PS12, Line 563: download_format profile_format -- 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: 12 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 02 Aug 2019 07:08:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4087/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 12 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 30 Jul 2019 23:32:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 642 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/12 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 12 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4035/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 11 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 26 Jul 2019 21:38:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4034/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 10 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 26 Jul 2019 21:34:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 642 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/11 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 11 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 10: (10 comments) http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@396 PS10, Line 396: /// “offset” : xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@397 PS10, Line 397: /// “events”: [{ tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@398 PS10, Line 398: /// “label”: xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@399 PS10, Line 399: /// “timestamp”: xxx tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@400 PS10, Line 400: /// },{...}] tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@451 PS10, Line 451: /// “counter_name” : xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@452 PS10, Line 452: /// “unit” : xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@453 PS10, Line 453: /// “num” : xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@454 PS10, Line 454: /// “period” : xxx, tab used for whitespace http://gerrit.cloudera.org:8080/#/c/13801/10/be/src/util/runtime-profile-counters.h@455 PS10, Line 455: /// “data”: “x,x,x,x” tab used for whitespace -- 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: 10 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 26 Jul 2019 20:54:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
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 10: (23 comments) Thanks for the valuable feedback! Looking forward to the next step and the version control discussion. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-hs2-server.cc@988 PS9, Line 988: } else { > nit: missing blank space after } Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@101 PS9, Line 101: #include "common/names.h" : : using boost::adopt_lock_t; : using boost::algorithm::is_an > Why not group these together with other rapidjson #include above ? There se Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@783 PS9, Line 783: turn Status::OK(); > DCHECK_EQ(format, TRuntimeProfileFormat::STRING); Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@818 PS9, Line 818: > DCHECK_EQ(format, TRuntimeProfileFormat::STRING); Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@126 PS9, Line 126: string CounterType() const override { > void ToJson(rapidjson::Document& document, rapidjson::Value* val) const ove Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@131 PS9, Line 131: > Actually, do you need to output this in json ? It seems sufficient to just Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@165 PS9, Line 165: private: : SampleFunction counter_fn_; : }; : : /// An AveragedCounter maintains a set of counters and its value is the : /// average of the values in that set. The average is updated through calls : /// > If you take the suggestion in Counter::ToJson(), you probably don't need th Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@219 PS9, Line 219: private: : /// Map from counters to their existing values. Modified via UpdateCounter(). : boost::unordered_map counter > See comments in Counter::ToJson() on why we may not need this. Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@222 PS9, Line 222: : /// Current sums of values from counter_value_map_. Only one of these is used, : /// depending on the unit of the counter. current_double_sum_ is used for : /// DOUBLE_VALUE, current_int_sum_ otherwise. : dou > Same question as above. Do you need to store the internal states (i.e. sum) Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@290 PS9, Line 290: /// Summary statistics of values seen so far. > virtual override Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@292 PS9, Line 292: t64_t max_; > Why is "kind" removed ? May be worth adding a comment here. Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@406 PS9, Line 406: void SortEvents() { > Please add a comment about the output format. Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@452 PS9, Line 452: /// “unit” : xxx, > It'd be nice to add a comment about the output format. Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@583 PS9, Line 583: : void Add(int64_t delta) override { : DCHECK(false); : } : : string CounterType() const override { : > Similar to comments above, this may not be needed since Counter::ToJson() m Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile.h@123 PS9, Line 123: counter_name, value, kind, unit > is this list actually accurate ? What about "kind" and "unit" ? Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile.cc@709 PS9, Line 709: } > nit: unnecessary blank line Done http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile.cc@797 PS9, Line 797: Value event_sequences_rapid(kArrayType); > nit:
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Future compatibility: The schema of the JSON format can be changed in the future with the standardization of Profile and Counter structure. Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 642 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/10 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 10 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 9: (23 comments) http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-hs2-server.cc@988 PS9, Line 988: }else { nit: missing blank space after } http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@101 PS9, Line 101: #include : #include : #include : #include Why not group these together with other rapidjson #include above ? There seems to be some duplicates. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@783 PS9, Line 783: DCHECK(format == TRuntimeProfileFormat::STRING); DCHECK_EQ(format, TRuntimeProfileFormat::STRING); http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/service/impala-server.cc@818 PS9, Line 818: DCHECK(format == TRuntimeProfileFormat::STRING); DCHECK_EQ(format, TRuntimeProfileFormat::STRING); http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@126 PS9, Line 126: void ToJson(rapidjson::Document& document, rapidjson::Value* val) const { void ToJson(rapidjson::Document& document, rapidjson::Value* val) const override { http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@131 PS9, Line 131: val->AddMember("current_value", current_value(), document.GetAllocator()); Actually, do you need to output this in json ? It seems sufficient to just call Counter::ToJson() which should add the higher water mark stored in "value_" ? "current_value_" seems to be internal state used for supporting TryAdd() only. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@165 PS9, Line 165: void ToJson(rapidjson::Document& document, rapidjson::Value* val) const { : Counter::ToJson(document, val); : rapidjson::Value::MemberIterator type_itr = val->FindMember("kind"); : DCHECK(type_itr != val->MemberEnd()); : type_itr->value.SetString("DerivedCounter"); : val->AddMember("counter_fn_val", counter_fn_(), document.GetAllocator()); : } If you take the suggestion in Counter::ToJson(), you probably don't need this function. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@219 PS9, Line 219: rapidjson::Value::MemberIterator type_itr = val->FindMember("kind"); : DCHECK(type_itr != val->MemberEnd()); : type_itr->value.SetString("AveragedCounter"); See comments in Counter::ToJson() on why we may not need this. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@222 PS9, Line 222: if (unit_ == TUnit::type::DOUBLE_VALUE){ : val->AddMember("current_double_sum", current_double_sum_, document.GetAllocator()); : } else { : val->AddMember("current_int_sum", current_int_sum_, document.GetAllocator()); : } Same question as above. Do you need to store the internal states (i.e. sum) in the json output object ? Isn't "average" the only state we need ? http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@290 PS9, Line 290: void ToJson(rapidjson::Document& document, rapidjson::Value* val) const { virtual override http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@292 PS9, Line 292: val->RemoveMember("kind"); Why is "kind" removed ? May be worth adding a comment here. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@406 PS9, Line 406: void ToJson(rapidjson::Document& document, rapidjson::Value* value); Please add a comment about the output format. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@452 PS9, Line 452: void ToJson(rapidjson::Document& document, rapidjson::Value* val); It'd be nice to add a comment about the output format. http://gerrit.cloudera.org:8080/#/c/13801/9/be/src/util/runtime-profile-counters.h@583 PS9, Line 583: void ToJson(rapidjson::Document& document, rapidjson::Value* val) const { : Counter::ToJson(document, val); : rapidjson::Value::MemberIterator type_itr = val->FindMember("kind"); : type_itr->value.SetString("ConcurrentTimerCounter"); : val->AddMember("concurrent_stop_watch_total_time",
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3991/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 25 Jul 2019 03:01:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
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 9: (4 comments) Done, thanks for the feedback. I think we need more discussion on the version control thing as Andrew and Bharath mentioned. http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h@292 PS8, Line 292: val->RemoveMember("kind"); > Should we not update the kind ? Why remove it? SummaryStatsCounter and TimeSeriesCounter are treated separately with normal counters in our JSON output. So a field like "kind" would be redundant. “counters”: [ { “counter_name”: xxx, “value”: xxx, “kind”: xxx, “unit”: xxx, “child_counters”: [{...}]// same structure as counter … //type special values },{...} ], “summary_stats_counters” : [ { “counter_name”: xxx, “value”: xxx, “unit”: xxx, “min”: xxx, “max”: xxx “avg”: xxx “num_of_samples”: xxx },{...} ] The difference as you can see from the schema, it's that we already know all the counters from summary_stats_counters belongs to SummaryStatsCounter. So it would not be necessary to keep the "kind" field again. http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@668 PS8, Line 668: > This looks flaky. Instead assert that they exist before you can loop throug Done http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@670 PS8, Line 670: if ("child_profiles" not in json_res["contents"]) or \ > dump the json_res if the assert fails, helps with debugging test failures. Done http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py@594 PS8, Line 594: assert False, "Download JSON format query profile cannot be parsed. " \ > dump json 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: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 25 Jul 2019 02:20:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 620 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/9 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 8: Should the json include the explicit version number of the format? "version: "1.0" to help people writing parsers? Like Bharath (and DavidR in the design doc) I would like to see more about future compatibility guarantees -- 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: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Mon, 22 Jul 2019 17:50:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 8: Code-Review+1 (5 comments) Some minor comments, but generally looks good to me. I spoke to Michael Ho offline who is going to take another look into the change and take it to a +2. Thanks for your patience so far. http://gerrit.cloudera.org:8080/#/c/13801/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13801/8//COMMIT_MSG@22 PS8, Line 22: Add a section on future compatibility guarantees? I think the idea is that this can evolve over time until we standardize the counters and the profile structure. So, until then if there are some Impal a changes, clients can expect the JSON structure to change too (keys could be added, deleted etc.) http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h@292 PS8, Line 292: val->RemoveMember("kind"); Should we not update the kind ? Why remove it? http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@668 PS8, Line 668: json_res["contents"]["child_profiles"][0]["info_strings"]: This looks flaky. Instead assert that they exist before you can loop through them? http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@670 PS8, Line 670: assert statement in info_string["value"] dump the json_res if the assert fails, helps with debugging test failures. http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py@594 PS8, Line 594: self.fail("Download JSON format query profile cannot be parsed.") dump json -- 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: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 19 Jul 2019 17:20:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3885/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 16 Jul 2019 00:31:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3884/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 16 Jul 2019 00:30:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
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 8: (17 comments) Thanks so much for the valid feedback. Have adopted the changes you suggested. Also add some more unit tests. Let me know if the code need more changes. Thanks! http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc@270 PS6, Line 270: if (format != TRuntimeProfileFormat::JSON){ > nit: Add a quick comment why? Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@126 PS6, Line 126: > nit: space const { . Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@128 PS6, Line 128: rapidjson::Value::MemberIterator type_itr = val->FindMember("kind"); > Add DHCECK (type_itr != val->MemberEnd())? (multiple places below too) Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@164 PS6, Line 164: > same (multiple places below) Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@289 PS6, Line 289: > why this? I am moving this away because SummaryStatsCounter and TimeSeriesCounter are all separate in our output JSON schema. “counters”: [ { “counter_name”: xxx, “value”: xxx, “kind”: xxx, “unit”: xxx, “child_counters”: [{...}]// same structure as counter … //type special values },{...} ], “summary_stats_counters” : [ { “counter_name”: xxx, “value”: xxx, “unit”: xxx, “min”: xxx, “max”: xxx “avg”: xxx “num_of_samples”: xxx },{...} ] I feel like it is a redundant content if we add its "kind". What do you think? I can always add it back if you feel that's better. http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@403 PS6, Line 403: > move to cc file? Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@421 PS6, Line 421: EventList events_; > instantiate value = ...directly in L407? I am not sure what to do with this... I am using the same schema as metric collection https://github.com/apache/impala/blob/95a1da2d32ea7b28585ad574b22c2bb9dd921029/be/src/util/collection-metrics.cc#L28 http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1173 PS6, Line 1173: > check other units too and assert the unit data? Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1187 PS6, Line 1187: // Serialize to json > some tests for info_strings and event_sequences too? Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1202 PS6, Line 1202: // Check counter value matches > check other TimeSeriesCounter implementations too? Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@705 PS6, Line 705: ); > remove rapidjson classifiers with using... Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@713 PS6, Line 713: CounterM > use d->GetAllocator() (avoid temp variable) Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@735 PS6, Line 735: } > formatting Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1590 PS6, Line 1590: lock_guard lock(lock_); > formatting Done http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1600 PS6, Line 1600: > formatting Done http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py@659 PS6, Line 659: get_profile_req.format = 3 # json format > why not do this in the above test after L654? Yeah, I can put this inside the above test. http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py@585 PS6, Line 585: # the query is in the file. > merge with test_download_text_profile? You can do something like 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
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Hello Bharath Vissapragada, Greg Rahn, Quanlong Huang, David Rorke, David Knupp, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13801 to look at the new patch set (#8). Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 611 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/8 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/7/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/13801/7/be/src/util/runtime-profile-test.cc@1302 PS7, Line 1302: line has trailing whitespace -- 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: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Mon, 15 Jul 2019 23:51:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Hello Bharath Vissapragada, Greg Rahn, Quanlong Huang, David Rorke, David Knupp, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13801 to look at the new patch set (#7). Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Tests: E2E tests: tests/webserver/test_web_pages.py - test_download_profile * merge text and json format download together HS2 tests: tests/hs2/test_hs2.py - test_get_profile * add json format test BE Unit tests: be/src/util/runtime-profile-test.cc * ToJson.RuntimeProfileToJsonTest * ToJson.EmptyTest * ToJson.EventSequenceToJsonTest * ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 611 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/7 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc@979 PS2, Line 979: if (request.format == TRuntimeProfileFormat::THRIFT) { : return_val.__set_thrift_profile(thrift_profile); : } else { : DCHECK(request.format == TRuntimeProfileFormat::STRING : || request.format == TRuntimeProfileFormat::BASE64); : return_val.__set_profile(ss.str()); : } > not sure; I'm not too familiar with this part of the code, I guess it depen I don't think we need to fix this for beeswax server as it will be deprecated in the future, especially given that shell now supports HS2 protocol. -- 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: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Mon, 15 Jul 2019 22:43:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc@979 PS2, Line 979: if (request.format == TRuntimeProfileFormat::THRIFT) { : return_val.__set_thrift_profile(thrift_profile); : } else { : DCHECK(request.format == TRuntimeProfileFormat::STRING : || request.format == TRuntimeProfileFormat::BASE64); : return_val.__set_profile(ss.str()); : } > Done. I am putting the output inside the profile, which is a string. not sure; I'm not too familiar with this part of the code, I guess it depends on when this method is invoked vs. the equivalent in impala-beeswax-server -- 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: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Mon, 15 Jul 2019 20:37:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 6: (17 comments) Some more minor comments, but generally lgtm. I'll let other reviewers take a look too. http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc@270 PS6, Line 270: if (format != TRuntimeProfileFormat::JSON){ nit: Add a quick comment why? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@126 PS6, Line 126: { nit: space const { . http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@128 PS6, Line 128: rapidjson::Value::MemberIterator type_itr = val->FindMember("kind"); Add DHCECK (type_itr != val->MemberEnd())? (multiple places below too) http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@164 PS6, Line 164: { same (multiple places below) http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@289 PS6, Line 289: val->RemoveMember("kind"); why this? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@403 PS6, Line 403: void ToJson(rapidjson::Document& document, rapidjson::Value* value) { move to cc file? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@421 PS6, Line 421: *value = event_sequence_rapid; instantiate value = ...directly in L407? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1173 PS6, Line 1173: counter_a = profile_a->AddCounter("A", TUnit::UNIT); check other units too and assert the unit data? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1187 PS6, Line 1187: EXPECT_TRUE(content.FindMember("info_strings")==content.MemberEnd()); some tests for info_strings and event_sequences too? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1202 PS6, Line 1202: TEST(ToJson, TimeSeriesCounterToJsonTest){ check other TimeSeriesCounter implementations too? http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@705 PS6, Line 705: rapidjson:: remove rapidjson classifiers with using... http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@713 PS6, Line 713: allocator use d->GetAllocator() (avoid temp variable) http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@735 PS6, Line 735: child_counter, counter_map,child_counter_map); formatting http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1590 PS6, Line 1590: document.GetAllocator()); formatting http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1600 PS6, Line 1600: document.GetAllocator()); formatting http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py@659 PS6, Line 659: def test_get_profile_json(self): why not do this in the above test after L654? http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py@585 PS6, Line 585: def test_download_json_profile(self): merge with test_download_text_profile? You can do something like for format in ("text", "json"): ... -- 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: 6 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Sat, 13 Jul 2019 04:00:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3859/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 6 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 11 Jul 2019 00:39:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
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 6: (1 comment) Thanks for your feedback! I also added some tests for this. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc@979 PS2, Line 979: SQLSTATE_GENERAL_ERROR); : if (request.format == TRuntimeProfileFormat::THRIFT) { : return_val.__set_thrift_profile(thrift_profile); : } else if (request.format == TRuntimeProfileFormat::JSON) { : rapidjson::StringBuffer sb; : rapidjson::PrettyWriter writer(sb); : > should there be an equivalent here for JSON? Done. I am putting the output inside the profile, which is a string. Also, do we need to adopt this change to impala-beeswax-server? I looked at the code there but find that the function there only support text format download. -- 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: 6 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 11 Jul 2019 00:05:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Hello Bharath Vissapragada, Greg Rahn, David Rorke, David Knupp, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13801 to look at the new patch set (#6). Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impalad profile page 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 4.Add tests for E2E test, unit test, hs2 test 5.Modify query profile page to different download option 6.Modify HS2 server to support get JSON format profile Tests: E2E tests: * tests/webserver/test_web_pages.py - test_download_json_profile HS2 tests: * tests/hs2/test_hs2.py - test_get_profile_json BE Unit tests: * be/src/util/runtime-profile-test.cc ToJson.RuntimeProfileToJsonTest ToJson.TimeSeriesCounterToJsonTest Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/CMakeLists.txt M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/hs2/test_hs2.py M tests/webserver/test_web_pages.py M www/query_profile.tmpl 16 files changed, 525 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/6 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 6 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc@979 PS2, Line 979: if (request.format == TRuntimeProfileFormat::THRIFT) { : return_val.__set_thrift_profile(thrift_profile); : } else { : DCHECK(request.format == TRuntimeProfileFormat::STRING : || request.format == TRuntimeProfileFormat::BASE64); : return_val.__set_profile(ss.str()); : } should there be an equivalent here for JSON? -- 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: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 03:58:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3854/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 5 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 02:22:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3853/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 4 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 02:16:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3852/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 3 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 02:13:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/5/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/5/tests/webserver/test_web_pages.py@608 PS5, Line 608: e flake8: E722 do not use bare except' -- 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: 5 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 01:42:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impala server 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 3.Add tests for E2E test 3.Modify query profile page to different download option Tests: E2E tests: * tests/webserver/test_web_pages.py - test_download_json_profile Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/RuntimeProfile.thrift M tests/webserver/test_web_pages.py M www/query_profile.tmpl 12 files changed, 405 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/5 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 5 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/13801/4/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/4/tests/webserver/test_web_pages.py@613 PS4, Line 613: j flake8: F841 local variable 'json_object' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/13801/4/tests/webserver/test_web_pages.py@614 PS4, Line 614: e flake8: F841 local variable 'e' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/13801/4/tests/webserver/test_web_pages.py@616 PS4, Line 616: flake8: W292 no newline at end of file -- 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: 4 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 01:36:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impala server 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 3.Add tests for E2E test 3.Modify query profile page to different download option Tests: E2E tests: * tests/webserver/test_web_pages.py - test_download_json_profile Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/RuntimeProfile.thrift M tests/webserver/test_web_pages.py M www/query_profile.tmpl 12 files changed, 410 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/4 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 4 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Description: Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Modification: 1.Add Json format download option in impala server 2.Add ToJson() function for RuntimeProfile, Counters, EventSequence 3.Add JSON format into QueryStateRecord 3.Add tests for E2E test 3.Modify query profile page to different download option Tests: E2E tests: * tests/webserver/test_web_pages.py - test_download_json_profile Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/RuntimeProfile.thrift M tests/webserver/test_web_pages.py M www/query_profile.tmpl 12 files changed, 410 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/3 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 3 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/13801/3/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/13801/3/tests/webserver/test_web_pages.py@613 PS3, Line 613: j flake8: F841 local variable 'json_object' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/13801/3/tests/webserver/test_web_pages.py@614 PS3, Line 614: e flake8: F841 local variable 'e' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/13801/3/tests/webserver/test_web_pages.py@616 PS3, Line 616: flake8: W292 no newline at end of file -- 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: 3 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 10 Jul 2019 01:32:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 2: (19 comments) Great work. Initial round of comments. Will take a deeper look in the next round. Lets also add some e-e tests to start with (look at test_web_pages.py). Feel free to add any unit tests in runtime-profile-test.cc. http://gerrit.cloudera.org:8080/#/c/13801/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13801/2//COMMIT_MSG@14 PS2, Line 14: Design Document: : https://docs.google.com/document/d/ : 15P_Lmjf1rlZUD4PZLXdUwTE8Lv_ME3NQ9Yf4MITzO2M/edit Lets get rid of this, might get stale, I'd suggest to add a PDF version to the jira. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.h File be/src/service/impala-http-handler.h: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.h@126 PS2, Line 126: json nit: lets stick to JSON (caps) for consistency everywhere. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc@266 PS2, Line 266: json_output_rapid curious why not use 'document' directly? http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc@272 PS2, Line 272: Value output_rapid(json_output_rapid["contents"], document->GetAllocator()); : document->AddMember("contents", output_rapid, document->GetAllocator()); : } else{ : document->AddMember("contents", rapidjson::StringRef(ss.str().c_str()), : document->GetAllocator()); see my comment below, lets not do this here. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc@283 PS2, Line 283: document->AddMember(rapidjson::StringRef(Webserver::ENABLE_RAW_HTML_KEY), true, why this? http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc@290 PS2, Line 290: document->AddMember(rapidjson::StringRef(Webserver::ENABLE_RAW_HTML_KEY), true, lets move this back to QueryProfileHelper http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-http-handler.cc@300 PS2, Line 300: document->RemoveMember("__common__"); Let's not do this to be consistent with other endpoints. I think we need to fix it properly in the webserver code for all of the text based content types. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.h@51 PS2, Line 51: nit: remove http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.cc@759 PS2, Line 759: rapidjson:: We pull the rapidjson ns above. We can get rid of this qualifier. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.cc@807 PS2, Line 807: } else if (format == TRuntimeProfileFormat::JSON) { Check formatting. Like in other cases, we need to handle the parse errors here. http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-server.cc@1981 PS2, Line 1981: rapidjson:: remove http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.h@25 PS2, Line 25: #include forward declare? http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.h@125 PS2, Line 125: rapidjson::Document* document general notation is to use output params as ptrs and other input params as const references. https://google.github.io/styleguide/cppguide.html#Output_Parameters http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.h@125 PS2, Line 125: { virtual void ToJson(...) const { http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.h@131 PS2, Line 131: } Lets move the function definition to the cpp file http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.cc@707 PS2, Line 707: rapidjson::Document::AllocatorType& allocator = d->GetAllocator(); nit: auto http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.cc@717 PS2, Line 717: rapidjson::Document* d, we use 4 space indents https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868536 http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/util/runtime-profile.cc@846 PS2, Line 846: lock_guard l(counter_map_lock_); lot of code
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3838/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 08 Jul 2019 18:12:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Design Document: https://docs.google.com/document/d/ 15P_Lmjf1rlZUD4PZLXdUwTE8Lv_ME3NQ9Yf4MITzO2M/edit Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/RuntimeProfile.thrift M www/query_profile.tmpl 11 files changed, 364 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/2 -- 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: newpatchset Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3821/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 1 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Jul 2019 22:09:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Jiawei Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13801 Change subject: IMPALA-5149: Provide query profile in JSON format .. IMPALA-5149: Provide query profile in JSON format Today there is a text and Thrift version of the query profile, but it would be useful to have a JSON version for portability and machine consumption. The ultimate goal is to have a Download JSON format profile link along with the other two formats. Design Document: https://docs.google.com/document/d/ 15P_Lmjf1rlZUD4PZLXdUwTE8Lv_ME3NQ9Yf4MITzO2M/edit Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/RuntimeProfile.thrift M www/query_profile.tmpl 11 files changed, 363 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13801/1 -- 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: newchange Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1 Gerrit-Change-Number: 13801 Gerrit-PatchSet: 1 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13801 ) Change subject: IMPALA-5149: Provide query profile in JSON format .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13801/1/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/13801/1/be/src/util/runtime-profile.cc@718 PS1, Line 718: const string& counter_name, const CounterMap& counter_map, line too long (99 > 90) -- 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: 1 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Jul 2019 21:29:52 + Gerrit-HasComments: Yes