Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21619 )

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


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.

http://gerrit.cloudera.org:8080/#/c/21619/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21619/1//COMMIT_MSG@14
PS1, Line 14: exist
nit: it exists.



--
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: 1
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-Comment-Date: Wed, 31 Jul 2024 22:43:01 +0000
Gerrit-HasComments: Yes

Reply via email to