Arun Suresh commented on YARN-3454:

Thanks for the patch [~curino].

Took a pass at the patch. Couple of comments :
# RLESparseResourceAllocation::merge (line 348) : Might be better if the 
(operator == subtract || subtractTestPositive) check were outside the for loop 
before calling the negateIfNeeded. maybe pass a boolean into the function (to 
signal throwing an exception if the val < 0)
# If the object of the ‘subtractTestPositive’ is to ensure that each component 
of the resource be >= 0, then this might not hold true if you are using the 
DominantResourceCalculator (since it compares only the dominant resource.. and 
the subtracting the other resource can lead to a negative number)

> RLESparseResourceAllocation does not handle removal of partial intervals (+ 
> introducing support for efficient "merge" operations) 
> ----------------------------------------------------------------------------------------------------------------------------------
>                 Key: YARN-3454
>                 URL: https://issues.apache.org/jira/browse/YARN-3454
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>    Affects Versions: 2.8.0, 2.7.1, 2.6.2
>            Reporter: Carlo Curino
>            Assignee: Carlo Curino
>         Attachments: YARN-3454.1.patch, YARN-3454.patch
> The RLESparseResourceAllocation.removeInterval(...) method handles well exact 
> match interval removals, but does not handles correctly partial overlaps. 
> In the context of this fix, we also introduced static methods to "merge" two 
> RLESparseResourceAllocation, while applying an operator in the process 
> (add/subtract/min/max/subtractTestPositive)

This message was sent by Atlassian JIRA

Reply via email to