[
https://issues.apache.org/jira/browse/TEZ-2690?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14702537#comment-14702537
]
Rajesh Balamohan commented on TEZ-2690:
---------------------------------------
lgtm overall. Minor comments.
- TezAnalyzerBase
-- Since the analyzer is not going to download the data, it might be good to
comment related to "DagId that needs to be downloaded".
-- Is the main() function needed in base class? Or is it given mainly as an
example?
-- Since base already extends Configured, Analyzer.getConfiguration() should be
removed. But this would be separate JIRA to let all analyzers extend
TezAnalyzerBase.
-- Printing usage might be useful (e.g need to refer to code for optional
parameter outputDir)
- Changes in VertexInfo is unintentional?
- SVGUtils - It might break the earlier drawVertex(DagInfo). But can be added
later as a part of refactoring other analyzers to extend TezAnalayzerBase.
- CriticalPathAnalyzer
-- getLastDataEventTime, getCreationTime etc got added as a part of TEZ-2701.
So if we try to parse with older logs (e.g 0.8/0.7/0.6 etc), it might return 0
for currentAttempt.getLastDataEventTime().
-- Should (currentAttempt.getLastDataEventTime() > 0) checks be added for such
cases to fail fast if the logs do not have those details? Other calculations
(e.g if (!Strings.isNullOrEmpty(currentAttempt.getLastDataEventSourceTA())))
can also become invalid. So it might be good to consider failing fast if the
logs do not have the info that analyzer is looking for.
Will go through the CriticalPathAnalyzer more in detail and post comments if
any.
> Add critical path analyser
> --------------------------
>
> Key: TEZ-2690
> URL: https://issues.apache.org/jira/browse/TEZ-2690
> Project: Apache Tez
> Issue Type: Sub-task
> Reporter: Bikas Saha
> Assignee: Bikas Saha
> Attachments: TEZ-2690.1.patch, criticalPath.jpg
>
>
> Use input and scheduling dependencies to create critical path for a DAG.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
