tez-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikas Saha (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (TEZ-1547) Make use of state change notifier in VertexManagerPlugins
Date Fri, 31 Oct 2014 19:44:34 GMT

    [ 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 7:44 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 
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


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.

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)

Mime
View raw message