[
https://issues.apache.org/jira/browse/TEZ-1547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14192318#comment-14192318
]
Bikas Saha edited comment on TEZ-1547 at 10/31/14 10:45 PM:
------------------------------------------------------------
bq. I think it'll be better to send this out as early as possible if it's
really a reconfiguration update
The difference between setParallelism and startVertex is 2 events on the
dispatcher queue that are sent back to back. setParallelism->immediately sends
READY_TO_INIT-> immediately triggers START-> sends CONFIGURED_COMPLETE. So time
difference would be negligibly small. Sending it in startVertex makes it future
proof to any code changes prior to start.
bq. Also, should we drop PARALLELISM_UPDATED - since that's a subset of
RECONFIGURED
IMO parallelism updated is useful by itself, specially when parallelism may be
updated multiple times. It can also be used to split downstream vertices on 1-1
edges instead of the current one-off implementation to do so. I had initially
thought of reconfigured but its not necessary that the vertex was reconfigured
in many case and so the name would be misleading.
bq. It's still possible for COMPLETELY_CONFIGURED to go out twice
Will fix.
bq. if (fromVertexManager && canInitVertex()) - Should probably be after the if
(parallelismSet check)
Will do.
bq. VertexManagerPlugin.onVertexStateUpdated (and
InputInitializerPluginContext.onVertexStateUpdated) - JavaDoc should ideally
mention
Will add.
bq. ImmediateStartVertexManager. Does it make sense to rename these
Will let the names stay since the behavior to start asap is still there.
bq. if (getContext().getVertexNumTasks(srcVertex) > 0)
Not having the check will work but I kept the original code because 0 task
vertices dont really need to be tracked.
bq. if (srcVertexConfigured.put(stateUpdate.getVertexName(),
true).booleanValue() == false) {
Duplicates should not occur. Will change that to a Precondition.
bq. Exception handling change in VertexImpl - the Vertex should move into
FAILED state instead of KILELD
Will do
bq. VertexManagerUserCodeErrorTransition needs to handle different states -
RUNNING (tasks scheduled) vs INITED/INITING
Will do.
bq. If leaving them in place, please add a comment on the variable indicating
eventual intent, for folks browsing the code.
Will do
was (Author: bikassaha):
bq. I think it'll be better to send this out as early as possible if it's
really a reconfiguration update
The difference between setParallelism and startVertex is 2 events on the
dispatcher queue that are sent back to back. setParallelism->immediately sends
READY_TO_INIT-> immediately triggers START-> sends CONFIGURED_COMPLETE. So time
difference would be negligibly small. Sending it in startVertex makes it future
proof to any code changes prior to start.
bq. Also, should we drop PARALLELISM_UPDATED - since that's a subset of
RECONFIGURED
IMO parallelism updated is useful by itself, specially when parallelism may be
updated multiple times. It can also be used to split downstream vertices on 1-1
edges instead of the current one-off implementation to do so. I had initially
thought of reconfigured but its not necessary that the vertex was reconfigured
in many case and so the name would be misleading.
bq. It's still possible for COMPLETELY_CONFIGURED to go out twice
Both the methods are under a writeLock(). So not sure how they can overlap with
each other. Can you please elaborate.
bq. if (fromVertexManager && canInitVertex()) - Should probably be after the if
(parallelismSet check)
Will do.
bq. VertexManagerPlugin.onVertexStateUpdated (and
InputInitializerPluginContext.onVertexStateUpdated) - JavaDoc should ideally
mention
Will add.
bq. ImmediateStartVertexManager. Does it make sense to rename these
Will let the names stay since the behavior to start asap is still there.
bq. if (getContext().getVertexNumTasks(srcVertex) > 0)
Not having the check will work but I kept the original code because 0 task
vertices dont really need to be tracked.
bq. if (srcVertexConfigured.put(stateUpdate.getVertexName(),
true).booleanValue() == false) {
Duplicates should not occur. Will change that to a Precondition.
bq. Exception handling change in VertexImpl - the Vertex should move into
FAILED state instead of KILELD
Will do
bq. VertexManagerUserCodeErrorTransition needs to handle different states -
RUNNING (tasks scheduled) vs INITED/INITING
Will do.
bq. If leaving them in place, please add a comment on the variable indicating
eventual intent, for folks browsing the code.
Will do
> Make use of state change notifier in VertexManagerPlugins
> ---------------------------------------------------------
>
> Key: TEZ-1547
> URL: https://issues.apache.org/jira/browse/TEZ-1547
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Siddharth Seth
> Assignee: Siddharth Seth
> Attachments: TEZ-1547.1.patch, TEZ-1547.3.patch, TEZ-1547.4.patch,
> TEZ-1547.5.patch, TEZ-1547.6.patch, TEZ-1547.7.patch, TEZ-1547.8.patch
>
>
> Instead of the various APIs like onVertexStarted, simple notifications could
> be sent.
> Some existing APIs could end up being deprecated.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
