[
https://issues.apache.org/jira/browse/MAPREDUCE-2933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098093#comment-13098093
]
Vinod Kumar Vavilapalli commented on MAPREDUCE-2933:
----------------------------------------------------
Looked at the patch, mostly looks good. Few comments, majority of them minor:
- RMAppAttemptContainerFinishedEvent can have ContainerStatus instead of
container.
- CapacityScheduler.java and FifoScheduler.java:
-- In {{removeNode()}} method, the status for the CompletedContainer can be
"Container on lost node" or "Lost node" or some such instead of "Lost
container". Thoughts?
-- {{createAbnormalContainerStatus()}} method can be shared between CS and
FifoScheduler. Either via {{BuilderUtils}} or a new {{SchedulerUtils}}.
- {{SchedulerApp.containerCompleted()}}: The containerStatus field that you so
laboriously passed around isn't finally used :) Actually you will still need
the conditional that was there before, except you send out the same event but
with different ContainerStatus objects. We need to make sure that diagnostics
and the state for Containers that really ran on the NM retain the remote
information.
- {{LeafQueue.assignReservedContainer()}}: For the completedContainer, we can
set set the diagnostics to be say "Reserved container no longer needed" or
something similar.
- {{ContainerImpl}}: Removal of "@SuppressWarnings("unchecked") // dispatcher
not typed". Though I don't prefer one way to the other, so far most of the
people whose MRV2 patches I happened to review seemed to have liked this, so
let's leave it like that, lest somebody add it again :)
- {{TestNodeStatusUpdater}}: Two assert statements on memory values (+152
pre-patch and +172 post-patch) seem to have been accidentally removed.
Similarly one more statement at +127 pre-patch(+149 post-patch).
> Change allocate call to return ContainerStatus for completed containers
> rather than Container
> ----------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2933
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2933
> Project: Hadoop Map/Reduce
> Issue Type: Sub-task
> Components: applicationmaster, mrv2, nodemanager, resourcemanager
> Reporter: Arun C Murthy
> Assignee: Arun C Murthy
> Priority: Blocker
> Fix For: 0.23.0
>
> Attachments: MAPREDUCE-2933.patch
>
>
> Change allocate call to return ContainerStatus for completed containers
> rather than Container, we should do this all the way from the NodeManager too.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
