Surya Hebbar 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 9: (1 comment) I agree with your opinion. It would be great to enhance EventSequence's constructor and the collection mechanism along with a cleanup and performance boost provided by switching to a completed version of profile V2. I was trying to mention something along those lines in the 3rd approach. But, also please take a look at the current patchset. In most profiles I had seen, missing events occurred in cases of medium to large number of nodes, and there always used to be at least one instance with a complete set of events. But still the problem of alignment and order occurred when a failed instance was first being used to generate the 'labels' map. This resulted in requiring a complete change in the order of all timestamps. After understanding the current collection mechanism, I have now addressed a big part of this problem by maintaining a reference to the reordering of labels themselves. So, now instead of all timestamps, only instances with missing timestamps need reordering. In case... 1. Some instances have missing events and there is at least a single instance with a complete event sequence, only those irregular instances are ordered and aligned efficiently. 2. There is no instance at all with a complete order, the initial ordering and alignment of labels and timestamps are kept, while skipping the inclusion of events at the end. 3. All instances are already in order, alignment and ordering is skipped with nearly zero overhead. This resolves almost all cases of missing timestamps with very less computation. I will still create a JIRA with the above example mentioning the unresolved corner cases and the possible improvement. Please let me know, if I am overlooking any cases with this approach. http://gerrit.cloudera.org:8080/#/c/21683/8/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/21683/8/be/src/util/runtime-profile.cc@97 PS8, Line 97: const std::vector<std::string> AggregatedRuntimeProfile::AGGREGATED_INFO_STRINGS_REQUIRED > line too long (107 > 90) Done -- 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: 9 Gerrit-Owner: Surya Hebbar <sheb...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Surya Hebbar <sheb...@cloudera.com> Gerrit-Comment-Date: Tue, 19 Nov 2024 20:12:08 +0000 Gerrit-HasComments: Yes
