[
https://issues.apache.org/jira/browse/MAPREDUCE-5124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16252724#comment-16252724
]
Jason Lowe commented on MAPREDUCE-5124:
---------------------------------------
Thanks for updating the patch! The approach looks good to me. Realize this is
POC, but I reviewed it as a "real" patch. Apologies if many of these were
already planned on being fixed in the real patch. Thought I'd get a head start
on that patch's review. ;-)
There's a bug here:
{code}
if (!lastStatusRef.compareAndSet(lastStatus, taskAttemptStatus)) {
// update failed - async dispatcher has processed it in the meantime
taskAttemptStatus.counters = oldCounters;
taskAttemptStatus.fetchFailedMaps = oldFetchFailedMaps;
asyncUpdatedNeeded = true;
}
{code}
compareAndSet doesn't set if the compare failed, so taskAttemptStatus never
gets posted to the reference and ultimately ends up going nowhere after it is
restored.
The listener should check if we fail to lookup the ref for the specified
attempt ID and throw an exception with a useful message.
Is there a reason not to use the YARN attempt ID as the key? That would
preclude the need to convert it on register and unregister, and it's already
being converted to the YARN record for other reasons during the status update
code.
The TaskAttemptListenerImpl#statusUpdate method could really use a refactor. I
recommend factoring out the portion that converts the TaskStatus into a
TaskAttemptStatus and separately factor out the portion that coalesces events
when necessary.
The TaskAttemptListener whitespace change isn't necessary.
The reportedTaskAttemptStatus field in TaskAttemptStatusUpdateEvent should be
removed. The code can have a constructor for the event that converts a status
to an atomic reference of the status if we don't want to update the existing
usages in the tests, but we shouldn't support anything but the reference in the
getter method and in the listener code. Way too confusing to continue exposing
two getter methods.
> AM lacks flow control for task events
> -------------------------------------
>
> Key: MAPREDUCE-5124
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-5124
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: mr-am
> Affects Versions: 2.0.3-alpha, 0.23.5
> Reporter: Jason Lowe
> Assignee: Peter Bacsko
> Attachments: MAPREDUCE-5124-CoalescingPOC-1.patch,
> MAPREDUCE-5124-CoalescingPOC2.patch, MAPREDUCE-5124-CoalescingPOC3.patch,
> MAPREDUCE-5124-proto.2.txt, MAPREDUCE-5124-prototype.txt
>
>
> The AM does not have any flow control to limit the incoming rate of events
> from tasks. If the AM is unable to keep pace with the rate of incoming
> events for a sufficient period of time then it will eventually exhaust the
> heap and crash. MAPREDUCE-5043 addressed a major bottleneck for event
> processing, but the AM could still get behind if it's starved for CPU and/or
> handling a very large job with tens of thousands of active tasks.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
