[
https://issues.apache.org/jira/browse/YARN-9318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16778600#comment-16778600
]
Daniel Templeton commented on YARN-9318:
----------------------------------------
Thanks for the patch, [~snemeth]. I have a few comments:
# Your {{applyFunctionOnValues(Resource lhs, Resource rhs, BiFunction<Long,
Long, Long> valueFunction)}} method adds complication for no gain. You're
still duplicating the logic, so you're better off leaving
{{multiplyAndAddTo()}} as it was.
# I'm kinda on the fence on the use of the lambdas. You only ever pass in two
lambdas: {{(value) -> (long) (value * by)}} and {{(value) -> (long)
Math.ceil(value * by)}}. The alternative would be to pass in a boolean that
says whether to round up or down. It would be easier to read but less
flexible. I don't see the multiplication functions changing that much in the
future, so I would suggest the boolean route.
# You've changed the behavior of the {{multiply*()}} methods. In the original
code, if a resource type causes an error, the operation aborts. In the new
code it keeps going. I personally think both behaviors are wrong; I think the
exception should be ducked. Regardless, let's not change behavior arbitrarily.
# In {{TestResources}} you should fix the name of {{testMultipleRoundUp()}} to
be {{testMultiplyRoundUp()}}.
# In {{testMultiplyAndRoundUpCustomResources()}} it would be great to have some
assert messages. While you're at it, it would also be nice to have more useful
assert messages in {{testMultipleRoundUp()}}.
> Resources#multiplyAndRoundUp does not consider Resource Types
> -------------------------------------------------------------
>
> Key: YARN-9318
> URL: https://issues.apache.org/jira/browse/YARN-9318
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Szilard Nemeth
> Assignee: Szilard Nemeth
> Priority: Major
> Attachments: YARN-9318.001.patch, YARN-9318.002.patch
>
>
> org.apache.hadoop.yarn.util.resource.Resources#multiplyAndRoundUp only deals
> with memory and vcores while computing the rounded value. It should also
> consider custom Resource Types as well.
--
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
