[
https://issues.apache.org/jira/browse/TEZ-776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14522738#comment-14522738
]
Bikas Saha commented on TEZ-776:
--------------------------------
bq. Can the fields in DataMovementEvent be made final after the new create
methods ?
The non-final ints are being set in multiple places. So this will have to wait.
bq. InputFailedEvent.makeCopy javadoc incomplete.
Fixed.
Ignoring comments for the obsolete version of the API.
bq. Unrelated to this jira, but a minor enhancement to the LOG the type of edge
during setup
Added
bq. This is not necessarily sufficient to determine whether ODR should be
enabled for the edge
Partially implementing the API will not work since this enables on demand
routing for all types of events. Like I said, the point of this check is not
identify ODR plugins but to identify legacy plugins. This is only there to
allow older versions of Hive to run with newer versions of Tez transparently.
This check is practically sufficient.
bq. Does this change the caching of events which made Broadcast and OneToOne
efficient earlier
No. This is a bug in the current code which is being rectified. We should not
change the original events received by the AM. If the same event is routed to
different tasks with different indices then all of them will end up seeing the
last index. Its was a bug waiting to be hit by the first edge plugin that would
do so.
bq. Think Hitesh already pointed this out, but a single event can explode into
multiple events - bigger than the maxEvents limit. Simple fix would be to just
accept the explosion and ignore the maxEvent limitation in this case.
.7 patch addresses that issue. Ignoring the limit is not an option. If too many
events are sent on the RPC then we can end up overloading the RPC and cause
other issues.
bq. In Edge, CompositeDataMovementEvent has it's own try catch throw
AMUserCodeException
.7 patch fixed it
bq. VerexImpl.getTaskAttemptTezEvents - Is taskEvents.size(), and any access to
taskEvents thread safe
The code is being optimistic here and its explained in the comments. Nothing is
deleted from this list and only one place for additions. So there is no
concurrent modification issues. Its ok to read the size outside the lock since
we will get a snapshot number to events to look at. Taking a lock is not going
to give any better guarantees about seeing the latest event being added.
bq. ROOT_INPUT_DATA_INFORMATION_EVENTS - Sending these directly can cause the
maxEventsPerHeartbeat to be exceeded
They are being guarded by the same overflow checks that guard the other events.
So it is fine.
bq. This can cause the number of events returned to the task to be lower than
maxEventsToGet.
.7 patch makes sure the events are fully packed to maxEvents
bq. Add events for the ones which support ODR to the vertex event list, hand
off the rest to the task
This would need to change the index based lookup from a task to change to
multiple indices instead of single index. That is a change I plan to make in a
follow up jira. This would enable separating edge routing from non-edge routing
or separate routing for different edges. Though I don't want to support the
existing legacy routing in tandem with ODR. Unnecessary complexity and ODR
offloads event processing from the central dispatcher which is a good benefit
by itself.
bq. I don't think ODR needs to be added to the OneToOne and Broadcast edges
That is not optimal. In the average small job case, the CPU is not very
relevant. However, for a large vertex with mixed edges we should not fall back
to legacy routing because the broadcast is legacy. In addition it take routing
pressure of the central dispatcher which helps get more useful work done faster
on the central dispatcher. In various scenarios the central dispatcher has
often shown up as a bottleneck. A lot of the CPU overhead will go away once we
stop creating new event objects in the AM. Thats another follow up jira to this
one.
> Reduce AM mem usage caused by storing TezEvents
> -----------------------------------------------
>
> Key: TEZ-776
> URL: https://issues.apache.org/jira/browse/TEZ-776
> Project: Apache Tez
> Issue Type: Sub-task
> Reporter: Siddharth Seth
> Assignee: Bikas Saha
> Attachments: TEZ-776.1.patch, TEZ-776.2.patch, TEZ-776.3.patch,
> TEZ-776.4.patch, TEZ-776.5.patch, TEZ-776.6.A.patch, TEZ-776.6.B.patch,
> TEZ-776.7.patch, TEZ-776.ondemand.1.patch, TEZ-776.ondemand.2.patch,
> TEZ-776.ondemand.3.patch, TEZ-776.ondemand.4.patch, TEZ-776.ondemand.5.patch,
> TEZ-776.ondemand.6.patch, TEZ-776.ondemand.7.patch, TEZ-776.ondemand.patch,
> With_Patch_AM_hotspots.png, With_Patch_AM_profile.png,
> Without_patch_AM_CPU_Usage.png, events-problem-solutions.txt,
> with_patch_jmc_output_of_AM.png, without_patch_jmc_output_of_AM.png
>
>
> This is open ended at the moment.
> A fair chunk of the AM heap is taken up by TezEvents (specifically
> DataMovementEvents - 64 bytes per event).
> Depending on the connection pattern - this puts limits on the number of tasks
> that can be processed.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
