Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21683 )
Change subject: IMPALA-13304: Include aggregate instance-level metrics within experimental profile(V2) ...................................................................... Patch Set 13: (13 comments) I think BE code coverage can be improved. http://gerrit.cloudera.org:8080/#/c/21683/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21683/12//COMMIT_MSG@130 PS12, Line 130: ", > Done Done http://gerrit.cloudera.org:8080/#/c/21683/13//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21683/13//COMMIT_MSG@78 PS13, Line 78: When no. of instances > 5 - : { : profile_name : <PLAN_NODE_NAME>, : num_children : <NUM_CHILDREN> : node_metadata : <NODE_METADATA_OBJECT> : event_sequences : : [{ : events : // An example event : [{ : label : "Open Started"" : ts_stat : : { : min : [ 2257887941, ...4 other division's minimum timestamps ], : max : [ 3257887941, ...4 other division's maximum timestamps ], : avg : [ 2757887941, ...4 other division's average timestamps ] : count : [ 2, ...4 other counts of division's no. of instances ] : } : }, ...other plan node's events : ] : }], : counters : <COUNTERS_OBJECT_ARRAY>, : child_profiles : <CHILD_PROFILES> : } Add test in runtime-profile-test.cc to verify creation of this structure. http://gerrit.cloudera.org:8080/#/c/21683/13//COMMIT_MSG@103 PS13, Line 103: { : profile_name : <PLAN_NODE_NAME>, : num_children : <NUM_CHILDREN> : node_metadata : <NODE_METADATA_OBJECT> : event_sequences : : [{ : offset : 0 : events : // An example event : [{ : label : "Open Started"" : ts_list : [ 2257887941, ...4 other instance's timestamps ] : }, ...other plan node's events : ] : }], : counters : <COUNTERS_OBJECT_ARRAY>, : child_profiles : <CHILD_PROFILES> : } Add test in runtime-profile-test.cc to verify creation of this structure. http://gerrit.cloudera.org:8080/#/c/21683/13//COMMIT_MSG@126 PS13, Line 126: : { : "info_strings" : : [{ : "key": "<info string's key>", : "values": [<distinct info string values>] : }] : } Add test in runtime-profile-test.cc to verify creation of this structure. http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc@1912 PS13, Line 1912: plan_node_profile["event_sequences"] assert that there is only 1 element in plan_node_profile["event_sequences"]. http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc@1914 PS13, Line 1914: events_json.Size() assert that events_json has the same size as NODE_LIFECYCLE_EVENTS. http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc@1915 PS13, Line 1915: events_json[i] For i > 0, can you validate that events_json[i]["ts_list"][j] > events_json[i - 1]["ts_list"][j] for all j? http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc@1981 PS13, Line 1981: plan_node_profile["event_sequences"] assert that there is only 1 element in plan_node_profile["event_sequences"]. http://gerrit.cloudera.org:8080/#/c/21683/13/be/src/util/runtime-profile-test.cc@1983 PS13, Line 1983: events_json.Size() assert that events_json has the same size as NODE_LIFECYCLE_EVENTS. http://gerrit.cloudera.org:8080/#/c/21683/12/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/21683/12/be/src/util/runtime-profile.cc@2733 PS12, Line 2733: : void AggregatedRuntimeProfile::PrettyPrintSubclassCounters( : ostream* s, const string& prefix, Verbosity verbosity) const { : ostream& stream = *s; : // Legacy profile did not show aggregated time series counters. : // Showing a time series per instance is too verbose for the default mode, : / > Done Done http://gerrit.cloudera.org:8080/#/c/21683/12/be/src/util/runtime-profile.cc@3138 PS12, Line 3138: > The structure has been kept similar to the traditional profile. Ack http://gerrit.cloudera.org:8080/#/c/21683/12/be/src/util/runtime-profile.cc@3149 PS12, Line 3149: > Done Done http://gerrit.cloudera.org:8080/#/c/21683/13/testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_v2.expected.pretty.json File testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_v2.expected.pretty.json: http://gerrit.cloudera.org:8080/#/c/21683/13/testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_v2.expected.pretty.json@2122 PS13, Line 2122: "label": "Prepare Finished", : "ts_stat": { : "min": [1121145, 12230160, 21292017, 34481634, 42591345], : "max": [9319969, 13196927, 21292017, 34481634, 50498895], : "avg": [5902216.75, 12734181.0, 21292017.0, 34481634.0, 45227210.666666667], : "count": [4, 3, 1, 1, 3] : } : }, { : "label": "Open Finished", : "ts_stat": { : "min": [1180328779, 0, 1183114930, 0, 1186253484], : "max": [1181173772, 0, 1183813349, 0, 1187280439], : "avg": [1180580685.75, 0, 1183396700.3333333, 0, 1186635686.2], : "count": [4, 0, 3, 0, 5] : } Why "count" for "Prepare Finished" does not match/align with "Open Finished"? Will it impact your visualization UI later? -- To view, visit http://gerrit.cloudera.org:8080/21683 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49e18a7a7e1288e3e674e15b6fc86aad60a08214 Gerrit-Change-Number: 21683 Gerrit-PatchSet: 13 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Tue, 17 Dec 2024 20:33:17 +0000 Gerrit-HasComments: Yes
