Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21683 )
Change subject: IMPALA-13304: Include aggregate instance-level metrics in JSON profile ...................................................................... Patch Set 19: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/21683/17/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/21683/17/be/src/util/runtime-profile-test.cc@2015 PS17, Line 2015: _event_probability(g > Done Done http://gerrit.cloudera.org:8080/#/c/21683/17/be/src/util/runtime-profile-test.cc@2251 PS17, Line 2251: > I thought "Grouped" would be more meaningful than NonAggregated. Ack. I misunderstand the name then. Everything is Grouped, but Aggregation only happen if BUCKET_SIZE < NUM_PROFILES. Are these names better reflect the tests? CompleteEvents_GroupedAndNotAggregatedCase CompleteEvents_GroupedAndAggregatedCase http://gerrit.cloudera.org:8080/#/c/21683/17/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/21683/17/be/src/util/runtime-profile.cc@67 PS17, Line 67: "Sets the number of spans / buckets for grouping of event timestamps within" : " the aggregated JSON profile. For example, if the number of fragment" : " instances(N) reporting an event is more than(N > M) the set > The aggregated profile was always being included in the traditional JSON pr But all your implementation is under AggEventSequence::ToJson, and to my knowledge, AggEventSequence only produced if --gen_experimental_profile=true. See IMPALA-9382: part 2/3: aggregate profiles sent to coordinator. http://gerrit.cloudera.org:8080/#/c/21683/19/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/21683/19/be/src/util/runtime-profile.cc@69 PS19, Line 69: (N > M) nit: remove this. -- 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: 19 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: Wed, 29 Jan 2025 21:14:06 +0000 Gerrit-HasComments: Yes
