[
https://issues.apache.org/jira/browse/TEZ-2678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14681069#comment-14681069
]
Siddharth Seth commented on TEZ-2678:
-------------------------------------
bq. Typos in API - Curretn, localicty, others
Fixed
bq. Add diagnostic string wherever ContainerEndReason is used.
Done. Also for TaskEndReason
bq. TODO in ContainerLauncherContext - TEZ-2676
Likely done elsewhere. Consolidation of TODOs and jiras
bq. TaskEndReason lossy compared to YARN.
[~hitesh] - could you please elaborate on this. (Was this meant to be YARN or
some internal error reporting?)
bq. Cache the context in DAGImpl.getDefaultExecutionContext
Done
bq. TaskAttempt. TA_KILLED moves to KILL_IN_PROGRESS instead of KILLED
Moving to KILLED now
bq. TaskAttempt - add scheduleTime to history event
Deferring to jira which will include additional history fixes like where a task
is executing. TEZ-2709
bq. Exception propagation in ContainerLauncherRouter
Converted UnknownHost to an unchecked exception as well.
bq. AMNodeTracker calls super("AMNodeMap");
Fixed
bq. ContainerLauncherOperationBase - token abstraction
Tracked in TEZ-2702
bq. TaskCommunicator.java Rename unregisterRunningTaskAttempt to
registerTaskAttemptEnd (make it more consistent)
Tracked in TEZ-2678
bq. Post merge / just before merge: Rename ContainerLauncherImpl to
TezContainerLauncherImpl ? Make all the default implementation with prefix Tez.
Tracked in TEZ-2708
bq. Typo DagTypeConverters.convertServicePluginDescriptoToProto -->
DagTypeConverters.convertServicePluginDescriptorToProto (miss "r")
Fixed
bq. Verify VertexExecutionContext matches against the ServicePluginDescriptor
setup for the TezClient
Fixed
bq. Verify abortTask/cleanup are used correctly in TezTaskRunner
ceanpu is always called - which is the correct thing to do. (In both
TezTaskRunner and TezTaskRunner2)
bq. why would a task call canCommit while shutting down? Shouldn’t we throw an
exception anyway as it is not meant to be called during shutdown?
Looked at this some more. The shutdown could be a result of anything including
preemption. The task doesn't necessarily know that it has been asked to die
(race with canCommit invocations or whatever the task is doing). Sending back
an exception results in an unnecessary exception from the task. A false seems
much safer - and has been the approach we've used for TaskRunner as well.
bq. TaskCommunicatorContextImpl: Shouldn’t each plugin manage its own
containers? Or at least shouldn’t this query be done based on which launcher
plugin was being used for the given container? Likewise for containerAlive(). |
Try fixing this to be specific to the communicator.
Fixed
bq. setAccessible not required during construction of plugins.
Fixed
bq. remove “*” e,g, import org.apache.tez.common.asterisk;
Fixed
bq. typos: logErrorIngored, hearbeats, getCurretnDagName
Fixed
> Fix comments from reviews - part 1
> ----------------------------------
>
> Key: TEZ-2678
> URL: https://issues.apache.org/jira/browse/TEZ-2678
> Project: Apache Tez
> Issue Type: Sub-task
> Affects Versions: TEZ-2003
> Reporter: Siddharth Seth
> Assignee: Siddharth Seth
>
> Typos in API - Curretn, localicty, others
> Add diagnostic string wherever ContainerEndReason is used.
> TODO in ContainerLauncherContext - TEZ-2676
> TaskEndReason lossy compared to YARN.
> Cache the context in DAGImpl.getDefaultExecutionContext
> TaskAttempt. TA_KILLED moves to KILL_IN_PROGRESS instead of KILLED
> TaskAttempt - add scheduleTime to history event
> Exception propagation in ContainerLauncherRouter
> AMNodeTracker calls super("AMNodeMap");
> ContainerLauncherOperationBase - token abstraction
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
