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 <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jiawei Wang <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Comment-Date: Thu, 25 Jul 2019 02:20:35 +0000 Gerrit-HasComments: Yes
