[
https://issues.apache.org/jira/browse/TEZ-1914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14299295#comment-14299295
]
Siddharth Seth commented on TEZ-1914:
-------------------------------------
Comments on the patch.
- Most parameters in VertexManager can be made final for visibility guarantees.
That may include moving parts of initialize into the constructor.
- Shouldn't plugin.initialize(); also run within a VM thread instead of the
central dispatcher ?
- VertexImpl locking becomes important with the change since invocations into
VertexImpl are now on a different thread. There were a bunch of findbugs errors
related to VertexImpl synchronization. It'll be useful to go through the calls
into VertexImpl from VertexManagerPluginContext to ensure correct
synchronization / locking behaviour. scheduleTasks is one - that seems OK, but
i only took a cursory look.
- Could you provide some more details on the intent of CallableEvent,
CallableEventDispatcher, etc ? If this is just for testing, limiting the scope
to within the tests would be better. Also, setting up a separate thread even
within unit tests would simulate behaviour better.
- VertexManagerCallback.onFailure - does it make sense to mark the VM as
complete, so that queued events don't end up running ?
- execService.shutdownNow(); - should this be before the central dispatcher is
shutdown, to avoid potential events being generated to a shutdown dispatcher.
- The shutdownNow can cause the exception in onFailure to show up as a
CancelledException. There were issues around this in TezChild which still need
to be resolved - but is worth considering; if not now, later.
> VertexManager logic should not run on the central dispatcher
> ------------------------------------------------------------
>
> Key: TEZ-1914
> URL: https://issues.apache.org/jira/browse/TEZ-1914
> Project: Apache Tez
> Issue Type: Task
> Reporter: Bikas Saha
> Assignee: Bikas Saha
> Attachments: TEZ-1914.1.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
