Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21619 )

Change subject: [tools] Update plan-graph.py
......................................................................


Patch Set 4:

> Patch Set 1: Code-Review+1
>
> (1 comment)
>
> Thanks for working on this Riza! I wish I knew the existence of this tool 
> much earlier. :-)
>
> It seemed I was not able to use plan-graph.py on the current master branch 
> (https://github.com/apache/impala/commit/f5a15ad) to parse a query profile 
> consisting of runtime filters. Not very sure if I missed something. I saw the 
> following stack trace when I tried to do so. So it's a bit difficult for me 
> to tell what has been changed only from the jpg/pdf file converted from a dot 
> file that was created by this patch.
>
> Traceback (most recent call last):
>   File "./plan-graph_asf-master.py", line 634, in <module>
>     main()
>   File "./plan-graph_asf-master.py", line 630, in main
>     dp.parse(args.infile)
>   File "./plan-graph_asf-master.py", line 615, in parse
>     self.draw()
>   File "./plan-graph_asf-master.py", line 463, in draw
>     self.draw_vertices(node_alias)
>   File "./plan-graph_asf-master.py", line 350, in draw_vertices
>     ft = self.filter_table_map[v_id]
> KeyError: 'RF001'
>
> Except for a very minor comment on the commit message, I have following minor 
> suggestions that do not have to be addressed.
> - Does it make sense to also add the value in the column of 'Max Time' in the 
> dot file? Or you have already done so? I am asking this because even though I 
> added the option '--verbosity 3' I could not see the value of 'Max Time', 
> which sometimes would be helpful in identifying nodes where there were skews 
> if 'Avg Time' and 'Max Time' differ a lot.
> - It may be obvious for most people using this tool but it would be great if 
> we could add a code comment to note that for a hash join node, we distinguish 
> the build-side edge from the probe-side edge. The color the build-side edge 
> is red and the probe-side edge is black.

Thanks Fang-Yu for trying out this script. Patch set 3 should address you 
comments.


--
To view, visit http://gerrit.cloudera.org:8080/21619
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22b0188bba3eef120c3e4b0f48408088123c4650
Gerrit-Change-Number: 21619
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Wed, 31 Jul 2024 23:12:45 +0000
Gerrit-HasComments: No

Reply via email to