[ https://issues.apache.org/jira/browse/YARN-8851?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16664540#comment-16664540 ]
Zhankun Tang commented on YARN-8851: ------------------------------------ [~csingh] , Thanks for the review! {quote}1. {code:java} DeviceRegisterRequest register(); {code} This is misleading. {{register()}} would mean that the device plugin is registering itself. However, here we need some information from the device plugin. Maybe, it can be changed to something like {code:java} DeviceResourceInfo getDeviceResourceInfo() {code} {quote} Zhankun-> Yeah. Weiwei also mentioned this problem. "getDeviceResourceInfo" is also very good. Now we have two names for it. :) "DeviceRegisterRequest getDeviceResourceInfo" and "DeviceRegisterRequest getRegisterRequestInfo()". Maybe the later one is more acurate since the "DeviceRegisterRequest" may contains more info besides resource name & version we currently want? {quote}2. {code:java} DeviceRuntimeSpec onDevicesUse(Set<Device> allocatedDevices, String runtime); {code} If this is get the {{DeviceRuntimeSpec}}, then should it be called {{getDeviceRuntimeSpec()}} ? {quote} Zhankun-> That's a good idea. {quote}3. Since we have callback for devices released, do we also need a callback for devices allocated? {{void onDevicesAllocated(Set<Device> allocatedDevices)}} {quote} Zhankun-> We have another interface "DevicePluginScheduler" to do this. And one may ask the reason why it's two interfaces, the intention here is that this scheduler interface is optional. And the other one is a must. {code:java} /** * Called when allocating devices. The framework will do all device book keeping * and fail recovery. So this hook should only do scheduling based on available devices * passed in. This method could be invoked multiple times. * @param availableDevices Devices allowed to be chosen from. * @param count Number of device to be allocated. * @return a set of {@link Device} * */ Set<Device> allocateDevices(Set<Device> availableDevices, Integer count);{code} {quote}4. Just a suggestion about logging Use slf4j logging format since that's the framework we are using and it improves readability of logging stmts. eg. instead of {{LOG.info("Adapter of " + pluginClassName + " created. Initializing..");}} we can use {code:java} LOG.info("Adapter of {} created. Initializing..", pluginClassName);{code} {quote} Zhankun -> Yeah. I also noted that we're using slf4j here in this "ResourcePluginManager" instead of log4j. Will change it. > [Umbrella] A new pluggable device plugin framework to ease vendor plugin > development > ------------------------------------------------------------------------------------ > > Key: YARN-8851 > URL: https://issues.apache.org/jira/browse/YARN-8851 > Project: Hadoop YARN > Issue Type: New Feature > Components: yarn > Reporter: Zhankun Tang > Assignee: Zhankun Tang > Priority: Major > Attachments: YARN-8851-WIP2-trunk.001.patch, > YARN-8851-WIP3-trunk.001.patch, YARN-8851-WIP4-trunk.001.patch, > YARN-8851-WIP5-trunk.001.patch, YARN-8851-WIP6-trunk.001.patch, > YARN-8851-WIP7-trunk.001.patch, [YARN-8851] > YARN_New_Device_Plugin_Framework_Design_Proposal-3.pdf, [YARN-8851] > YARN_New_Device_Plugin_Framework_Design_Proposal.pdf > > > At present, we support GPU/FPGA device in YARN through a native, coupling > way. But it's difficult for a vendor to implement such a device plugin > because the developer needs much knowledge of YARN internals. And this brings > burden to the community to maintain both YARN core and vendor-specific code. > Here we propose a new device plugin framework to ease vendor device plugin > development and provide a more flexible way to integrate with YARN NM. -- 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