[ 
https://issues.apache.org/jira/browse/YARN-7192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16166341#comment-16166341
 ] 

Jason Lowe commented on YARN-7192:
----------------------------------

Took a quick look at the patch, some initial comments:

Would it make sense for the listener API to directly support multiple 
listeners?  Typically the types of APIs do, so a bit surprised to not see it 
here.  It can always be added later, but that requires a "multi-listener 
listener" class to be configured and yet another config to specify the list of 
actual listeners desired.  If multiple are supported then there's the question 
of whether events are always dispatched or if callbacks can suppress further 
notification ala POSIX signal handling, GUI event handling, and other similar 
setups.

Does a container listener require more than just a conf?  Wondering if it may 
need the ability to see the NMContext, the Nodemanager object, etc. to get 
access to other services within the NM to do what it wants to do.

The InternalStateMachine constructor that doesn't take a listener should just 
pass null to the one that does rather than duplicate the default setting.  That 
way there's only one place where we set the default listener, and if that needs 
to change there's only one place to update.

{code}
    try {
      ContainerStateTransitionListener listener =
          ReflectionUtils.newInstance(listenerClass, conf);
      nmContext.setContainerStateTransitionListener(listener);
    } catch (Exception e) {
      throw new RuntimeException(
          "Could not instantiate [" + listenerClass.getName() + "]");
    }
{code}
The exception that was caught is smashed with a generic RuntimeException 
because it doesn't pass it on as the cause.  This will make it very difficult 
to debug from the logs.


> Add a pluggable StateMachine Listener that is notified of NM Container State 
> changes
> ------------------------------------------------------------------------------------
>
>                 Key: YARN-7192
>                 URL: https://issues.apache.org/jira/browse/YARN-7192
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Arun Suresh
>            Assignee: Arun Suresh
>         Attachments: YARN-7192.001.patch
>
>
> This JIRA is to add support for a plugggable class in the NodeManager that is 
> notified of changes to the Container StateMachine state and the events that 
> caused the change.
> The proposal is to modify the basic StateMachine class add support for a hook 
> that is called before and after a transition.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to