Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/21593 )
Change subject: IMPALA-13233: Represent phases of each instance separately on the query timeline ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/21593/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21593/2//COMMIT_MSG@20 PS2, Line 20: When there are more than 4 instances for a phase, respective timestamps : are bucketed into 4 groups. Each group's timestamps are averaged and : then displayed on the timeline. > How is bucketing done here? Are they sorted first? The timestamps are not sorted, instead the average of timestamps in each bucket is plotted, and a tooltip is shown with number of instances, maximum and minimum timestamp. When there are large number of nodes, it would add considerable overhead to sort all instance's timestamps in each phase for all fragment's plan nodes. This was the possible alternative without adding a considerable delay to rendering. The methodology of collecting timestamps from profile has become quite complex with many conditionals, as there is a possibility of missing event timestamps with a large number of instances. So, during this process currently timestamp's 'instance/host name' is not collected from profile. After a mechanism to collect and segregate instance's event timestamps based on instance name is added, with this additional processing delay it would be possible to render based on host names(MT_DOP=TRUE). Though, even after adding this complexity, we would still need to display the same 'maximum', 'minimum' and 'no of instances' for each group. Hence, even in this case, the user cannot directly see the phase's precise termination timestamp and would be relying on the tooltip. I believe this would require further deliberation before implementation. http://gerrit.cloudera.org:8080/#/c/21593/2/www/scripts/query_timeline/fragment_diagram.js File www/scripts/query_timeline/fragment_diagram.js: http://gerrit.cloudera.org:8080/#/c/21593/2/www/scripts/query_timeline/fragment_diagram.js@167 PS2, Line 167: events > Can you put diagram/comment on how the structure of this events list is? Sure. Now, I have added comments explaining each 'events' element's structure. http://gerrit.cloudera.org:8080/#/c/21593/2/www/scripts/query_timeline/fragment_diagram.js@202 PS2, Line 202: bottom_edge_ts = events[last_e_in > Why events[0] is handled separately from other at L238? No, all other phases except the 1st phase begin from previous phase's terminal point. So, in all the other phases previous event's timestamps average needs to be retrieved. The less efficient alternative is adding a conditional statement. -- To view, visit http://gerrit.cloudera.org:8080/21593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied8a5966e9e4111bf7aa25aee11d23881daad7d2 Gerrit-Change-Number: 21593 Gerrit-PatchSet: 3 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: Thu, 01 Aug 2024 06:46:35 +0000 Gerrit-HasComments: Yes
