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

Eric Badger commented on YARN-10503:
------------------------------------

Thanks for the update, [~zhuqi]. This might be a little too picky, but I think 
it would be better if {{appendCustomResources}} just created the string instead 
of appending it to {{resourceString}}. That way we can keep the current 
structure of the StringBuilders at the caller level.

{noformat}
    resourceString
        .append("[" + AbsoluteResourceType.MEMORY.toString().toLowerCase() + "="
            + resource.getMemorySize() + ","
            + AbsoluteResourceType.VCORES.toString().toLowerCase() + "="
            + resource.getVirtualCores()
            + getCustomResourcesString(resource) + "]");
{noformat}
It could look something like this, where {{getCustomResourcesString}} returns 
the string instead of appending it. 

{noformat}
    // Custom resource type defined by user.
    // Such as GPU FPGA etc.
    if (!resourceTypes.contains(resourceName)) {
      resource.setResourceInformation(resourceName, ResourceInformation
          .newInstance(resourceName, units, resourceValue));
      return;
    }

    // map it based on key.
    AbsoluteResourceType resType = AbsoluteResourceType
        .valueOf(StringUtils.toUpperCase(resourceName));
    switch (resType) {
    case MEMORY :
      resource.setMemorySize(resourceValue);
      break;
    case VCORES :
      resource.setVirtualCores(resourceValue.intValue());
      break;
    default :
      resource.setResourceInformation(resourceName, ResourceInformation
          .newInstance(resourceName, units, resourceValue));
      break;
    }
  }
{noformat}
This snippet of code confuses me a bit. What's the purpose of thee initial if 
statement? If the resource doesn't already container the resource in question, 
we add it and then return. But in the case that it does exist, we go to the 
switch statement, add it, and then return. I looks like the if statement is 
unnecessary. Am I missing something?

> Support queue capacity in terms of absolute resources with custom 
> resourceType.
> -------------------------------------------------------------------------------
>
>                 Key: YARN-10503
>                 URL: https://issues.apache.org/jira/browse/YARN-10503
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Qi Zhu
>            Assignee: Qi Zhu
>            Priority: Critical
>         Attachments: YARN-10503.001.patch, YARN-10503.002.patch, 
> YARN-10503.003.patch, YARN-10503.004.patch, YARN-10503.005.patch, 
> YARN-10503.006.patch, YARN-10503.007.patch
>
>
> Now the absolute resources are memory and cores.
> {code:java}
> /**
>  * Different resource types supported.
>  */
> public enum AbsoluteResourceType {
>   MEMORY, VCORES;
> }{code}
> But in our GPU production clusters, we need to support more resourceTypes.
> It's very import for cluster scaling when with different resourceType 
> absolute demands.
>  
> This Jira will handle GPU first.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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