[
https://issues.apache.org/jira/browse/TEZ-1170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14032824#comment-14032824
]
Siddharth Seth commented on TEZ-1170:
-------------------------------------
bq. Every place is consistent to the extent that they all call
VertexInitializedTransition.doTransition(). But as you observe the timing is
different, new event vs direct transition. Direct transition is not possible in
the cases where setParallelism() results in initializing the parallelism since
that method may be called from within the Vertex Managers. So the
setParallelism case uses the READY_TO_INIT event approach to execute
VertexInitializedTransition. All cases where initialization is waiting for
setParallelism now consistently use the setParallelism method to complete the
initialization. This is VertexManagers setting parallelism - custom vertex
manager case, AM split generation input init case and source 1-1 split
handling. There are 2 other cases where we need to execute the
VertexInitializedTransition - when init is triggered because a null edge is
initialized and the AM split distributor case. In these cases setParallelism is
not called. I could make them consistent by having them send the READY_TO_INIT
event but that would put it on the event queue.
I hope that clarifies the reason the mixed event and direct transitions. If you
have any ideas on making setParallelism execute the transition directly then we
can try to unify them. Or we could have the direct transitions change to the
event route.
bq. I renamed it because I thought I would use the event approach consistently
in which case it would be sent for null edge init also and so the parallelism
name would be misleading. Also before the event is sent not only parallelism
but also other preconditions are checked like all edges defined etc. Thats why
associating the event with only parallelism would be misleading. How about
changing the name to V_INITIALIZATION_COMPLETE?
A setParallelism invocation isn't always sufficient to move the vertex over to
INITED state, right ? OTOH it is used to potentially setup edges, or
parallelism - in that sense, a single event (PARALLELISM(num_tasks)_SET /
EDGE_INITIALIZED) is not enough. Just a SET_PARALLELISM_INVOKED may be a better
event.
EDGE_INITIALIZED, ROOT_INPUT_INITIALIZED, SET_PARALLELISM_INVOKED would end up
with a common check to move the state machine to the next state.
Checking for other conditions in the setParallelism call is an optimization
(which is already wired into the state machine for EDGE_INITIALIZED,
ROOT_INPUT_INITIALIZED). The same could be applied to the ONE_TO_ONE transition
- since the setParallelism call is being invoked from within the framework code
in this case.
bq. I should have removed that transition. I put it there since I had a hunch
that we might have a race in our init where the edge may be initialized before
the vertex receives an init event.
This race may just be possible, if the source vertex has multiple incoming
edges - one of which is a 1-1 event. The source doesn't need to move into
INITED state (and send out an INIT event) before the OneToOne event is sent
forward, does it ?
bq. Yes. This is to ensure that shutdown is called on the initializers if the
vertex is say killed by the client after parallelism is set but some
initializers are not yet done. This might be a dead transition. If you agree
then we can remove it.
Don't think it can be removed, but should be ignored. The event can come in
since the Initializer is running in a separate thread, or could be backlogged
in the dispatcher queue. The shutdown though, I don't think is required, since
it should have already been invoked.
bq. So to be safe we wait for all intializers to finish when the initializers
are not determining parallelism.
Fair enough, similar to allowing source vertices to move into their required
state before initing the current vertex.
> Simplify Vertex Initializing transition
> ---------------------------------------
>
> Key: TEZ-1170
> URL: https://issues.apache.org/jira/browse/TEZ-1170
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Siddharth Seth
> Assignee: Bikas Saha
> Fix For: 0.5.0
>
> Attachments: TEZ-1170.1.patch, TEZ-1170.2.patch
>
>
> After TEZ-1145 and 1151, a vertex should only need to stay in INITIALZING
> state when it has an uninitialized edge, or when the parallelism is at -1
> (not set yet). Waiting for all RootInputInitializers to complete should not
> be required - as long as one of them sets parallelism (via a VertexManager).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
