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

Chandni Singh commented on YARN-8881:
-------------------------------------

Hi [~tangzhankun], Thanks for the patch. I have some questions/comments

1.
{code}       
if (!DevicePlugin.class.isAssignableFrom(pluginClazz)) {
        throw new YarnRuntimeException("Class: " + pluginClassName
            + " not instance of " + DevicePlugin.class.getCanonicalName());
      }
      // sanity-check before initialization
      checkInterfaceCompatibility(DevicePlugin.class, pluginClazz);
{code}
It checks if the pluginClazz is an implementation of {{DevicePlugin}} via 
{{isAssginableFrom}}. Why is {{checkInterfaceCompatibility}} required? 

2. Nitpick: DevicePluginAdapter ->  resourceName  and devicePlugin can be final.
   LOG -> can be private.
{code}
  final static Log LOG = LogFactory.getLog(DevicePluginAdapter.class);

  private String resourceName;
  private DevicePlugin devicePlugin;
{code}

3.
{code}
    LOG.debug("Checking implemented interface's compatibility: \"{}\"",
        expectedClass.getSimpleName());
{code}
  The braces {} don't need to be surrounded by quotes. It is surrounded by 
quotes at quite a few places.

4. serialVersionUID is 1 for all the serializable classes. Use a time based or 
random large number.

> Phase 1 - Add basic pluggable device plugin framework
> -----------------------------------------------------
>
>                 Key: YARN-8881
>                 URL: https://issues.apache.org/jira/browse/YARN-8881
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Zhankun Tang
>            Assignee: Zhankun Tang
>            Priority: Major
>         Attachments: YARN-8881-trunk.001.patch, YARN-8881-trunk.002.patch, 
> YARN-8881-trunk.003.patch, YARN-8881-trunk.004.patch, 
> YARN-8881-trunk.005.patch, YARN-8881-trunk.006.patch, 
> YARN-8881-trunk.007.patch, YARN-8881-trunk.008.patch, 
> YARN-8881-trunk.009.patch
>
>
> It includes adding support in "ResourcePluginManager" to load plugin classes 
> based on configuration, an interface for the vendor to implement and the 
> adapter to decouple plugin and YARN internals. And the vendor device resource 
> discovery will be ready after this support



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
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