[
https://issues.apache.org/jira/browse/TEZ-1914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14308873#comment-14308873
]
Bikas Saha commented on TEZ-1914:
---------------------------------
bq. Most parameters in VertexManager can be made final for visibility
guarantees.
The visibility should be fine since the construction happens much before any
thread processing. Anyways, I have made everything final.
bq. Shouldn't plugin.initialize(); also run within a VM thread instead of the
central dispatcher ?
That cannot be because the vertex manager plugin has until the initialize()
invocation to notify the framework about intent to reconfigure the vertex. If
this happens async then the vertex will move ahead without the notification.
bq. VertexImpl locking becomes important with the change since invocations into
VertexImpl are now on a different thread.
We are safe within the same vertex manager since the pluginContext methods are
synchronized. I have added readLocks for a few methods that werent locked. A
bunch of this is probably moot since the pluginContext is provided to the
vmPlugin in its constructor instead of the initializer() method. So a vmPlugin
could invoke plugin methods earlier than initialize() and get bogus values.
bq. Could you provide some more details on the intent of CallableEvent,
CallableEventDispatcher, etc ? If this is just for testing
For now yes, though I would like to explore the option of using these more
generally where a dispatcher could run callable events on a multi-threaded
executor.
IMO, the unit tests are designed to exercise specific code paths and
repeatability and debuggability is more important there. Hence I have chosen to
serialize them transparently in the setup methods so that existing and new
tests can be written with stability.
bq. VertexManagerCallback.onFailure - does it make sense to mark the VM as
complete, so that queued events don't end up running ?
Yes. Added new pluginFailed since complete represents the plugin has notified
us that its done.
bq. execService.shutdownNow(); - should this be before the central dispatcher
is shutdown, to avoid potential events being generated to a shutdown dispatcher.
The dispatcher is resilient to events generated after stopping.
bq. The shutdownNow can cause the exception in onFailure to show up as a
CancelledException.
In the AM, this would probably not be interesting since its after the AM has
shutdown.
bq. Is eventInFlight in VM used for don't consume events if failure happens ?
If so, make VM complete should be easier.
No. This is to ensure that events for a given vertex plugin are processed
sequentially. So if there is a large number of events for a given vertex then
it will not take over all the threads in the executor. Also, for now, this make
the plugin writers life simple since plugins dont have to worry about
multi-threading due to parallel event invocations.
bq. exception handling lstg.
lstg???
> 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)
