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

Zhankun Tang edited comment on YARN-8881 at 11/15/18 2:28 AM:
--------------------------------------------------------------

[~csingh], Thanks for the review!
{quote}It checks if the pluginClazz is an implementation of {{DevicePlugin}} 
via {{isAssginableFrom}}. Why is {{checkInterfaceCompatibility}} required?
{quote}
Zhankun => Good question. isAssiginableFrom only check whether the interface is 
the plugin class's parent. But it doesn't ensure the real methods implemented 
in the plugin class matches YARN needs. For instance, "FakeTestDevicePlugin4" 
implement a method "getOldRegisterRequestInfo"(should be 
"getRegisterRequestInfo") but can also be reflected and will throw an error 
until _plugin.getRegisterRequestInfo_ is called if we don't check. This could 
happen due to the plugin depend on an outdated NM API or some reason we don't 
know.

Here the _checkInterfaceCompatibility_ does a basic fast fail to ensure methods 
invoked by YARN exists. We'll add more sanity-check later to ensure plugin that 
its return value is correct, the method implemented is stateless .etc.
{quote}DevicePluginAdapter -> resourceName and devicePlugin can be final.
 LOG -> can be private.
{quote}
Zhankun => Thanks. Will fix it
{quote}The braces {} don't need to be surrounded by quotes. It is surrounded by 
quotes at quite a few places.
{quote}
Zhankun => Yeah. Will remove all quotes.
{quote}serialVersionUID is 1 for all the serializable classes. Use a time based 
or random large number.
{quote}
Zhankun => Yeah. Will change them.


was (Author: tangzhankun):
[~csingh], Thanks for the review!
{quote}It checks if the pluginClazz is an implementation of {{DevicePlugin}} 
via {{isAssginableFrom}}. Why is {{checkInterfaceCompatibility}} required?
{quote}
Zhankun => Good question. isAssiginableFrom only check whether the interface is 
the plugin class's parent. But it doesn't ensure the real methods implemented 
in the plugin class matches YARN needs. For instance, "FakeTestDevicePlugin4" 
implement a method "getOldRegisterRequestInfo"(should be 
"getRegisterRequestInfo") but can also be reflected and will throw an error 
until _plugin.getRegisterRequestInfo_ is called. This could happen due to the 
plugin depend on an outdated NM API or some reason we don't know.

Here the _checkInterfaceCompatibility_ does a basic fast fail to ensure methods 
invoked by YARN exists. We'll add more sanity-check later to ensure plugin that 
its return value is correct, the method implemented is stateless .etc.
{quote}DevicePluginAdapter -> resourceName and devicePlugin can be final.
 LOG -> can be private.
{quote}
Zhankun => Thanks. Will fix it
{quote}The braces {} don't need to be surrounded by quotes. It is surrounded by 
quotes at quite a few places.
{quote}
Zhankun => Yeah. Will remove all quotes.
{quote}serialVersionUID is 1 for all the serializable classes. Use a time based 
or random large number.
{quote}
Zhankun => Yeah. Will change them.

> 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