[
https://issues.apache.org/jira/browse/YARN-6885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16121942#comment-16121942
]
Daniel Templeton commented on YARN-6885:
----------------------------------------
Thanks for the patch, [~Yu-Tang Lin]. Here are my comments:
# I don't see the value of declaring {{text}} and {{val}} outside the _switch_.
If saves some characters, but it has no runtime impact, and I think it makes
the code a less clear, especially since you're having to cast {{val}} when
using it.
# In {code}312 case "queueMaxResourcesDefault":
313 text = ((Text)element.getFirstChild()).getData().trim();
314 val =
315
FairSchedulerConfiguration.parseResourceConfigValue(text);
316 queueMaxResourcesDefault = (Resource)val;
317 break;{code} lines 314 and 315 can be combined.
# In {code}362 if (text.equalsIgnoreCase(FifoPolicy.NAME)) {
363 throw new AllocationConfigurationException("Bad fair
scheduler "
364 + "config file: defaultQueueSchedulingPolicy or "
365 + "defaultQueueSchedulingMode can't be FIFO.");
366 }
367 defaultSchedPolicy = SchedulingPolicy.parse(text);
368 break;{code} lines 364 and 365 should be indented 4 more
spaces.
# In {code}545 case "weight":
546 {
547 text = ((Text)field.getFirstChild()).getData().trim();
548 double doubleVal = Double.parseDouble(text);
549 queueWeights.put(queueName, new
ResourceWeights((float)doubleVal));
550 }
551 break;{code} Why the braces? I don't think they're needed.
# Same here: {code}562 case "fairSharePreemptionThreshold":
563 {
564 text = ((Text)field.getFirstChild()).getData().trim();
565 float floatVal = Float.parseFloat(text);
566 floatVal = Math.max(Math.min(floatVal, 1.0f), 0.0f);
567 fairSharePreemptionThresholds.put(queueName, floatVal);
568 }
569 break;{code}
# In {{AllocationFileLoaderService}}, the last two _case_ statements don't
work, because the _if_ statements they're replacing weren't testing equality.
You'll have to keep the _if_ statements and put them in the _default_ case.
> AllocationFileLoaderService.loadQueue() should use a switch statement in the
> main tag parsing loop instead of the if/else-if/...
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: YARN-6885
> URL: https://issues.apache.org/jira/browse/YARN-6885
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: fairscheduler
> Affects Versions: 3.0.0-alpha4
> Reporter: Daniel Templeton
> Assignee: Yu-Tang Lin
> Priority: Minor
> Labels: newbie
> Fix For: 3.0.0-alpha4
>
> Attachments: YARN-6885.005.patch
>
>
> {code} if ("minResources".equals(field.getTagName())) {
> String text = ((Text)field.getFirstChild()).getData().trim();
> Resource val =
> FairSchedulerConfiguration.parseResourceConfigValue(text);
> minQueueResources.put(queueName, val);
> } else if ("maxResources".equals(field.getTagName())) {
> ...{code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
