mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 49190: Enabled calculateShare() to ignore the fairnessExcludeResourceNames.
Date Fri, 01 Jul 2016 00:45:13 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49190/#review140270
-----------------------------------------------------------


Fix it, then Ship it!





src/master/allocator/sorter/drf/sorter.hpp (lines 127 - 128)
<https://reviews.apache.org/r/49190/#comment205630>

    Let's move this up to be below the constructor/destructor before all the other methods
since this should be called first.



src/master/allocator/sorter/drf/sorter.hpp (line 138)
<https://reviews.apache.org/r/49190/#comment205629>

    How about:
    
    ```
    // Resources (by name) that will be excluded from fair sharing.
    ```



src/master/allocator/sorter/drf/sorter.cpp (line 412)
<https://reviews.apache.org/r/49190/#comment205633>

    // Filter out the resources excluded from fair sharing.



src/master/allocator/sorter/drf/sorter.cpp (line 414)
<https://reviews.apache.org/r/49190/#comment205632>

    s/.get()./->/
    
    We prefer to avoid implicit casting of integers as booleans, so the count would be explicitly
checked:
    
    ```
        if (fairnessExcludeResourceNames.isSome() &&
            fairnessExcludeResourceNames->count(scalar) > 0) {
          continue;
        }
    ```



src/master/allocator/sorter/drf/sorter.cpp (lines 456 - 459)
<https://reviews.apache.org/r/49190/#comment205631>

    -Brace on the next line
    -s/std:://
    -And let's move this up to be the first method.



src/master/allocator/sorter/sorter.hpp (lines 145 - 148)
<https://reviews.apache.org/r/49190/#comment205628>

    Let's move this up next to the constructor since the sorter should be initialized after
construction before anything else is called.
    
    We should make the comment less about the fairness exclusion argument and more about that
this initializes the sorter.


- Benjamin Mahler


On June 30, 2016, 7:59 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49190/
> -----------------------------------------------------------
> 
> (Updated June 30, 2016, 7:59 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5621
>     https://issues.apache.org/jira/browse/MESOS-5621
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enabled calculateShare() to ignore the fairnessExcludeResourceNames.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/sorter/drf/sorter.hpp 35273b5dcf39938125a112c5e56b5a8a542157db

>   src/master/allocator/sorter/drf/sorter.cpp 27d56f274c41bbabe6f2abbbcebd2c4eff52693e

>   src/master/allocator/sorter/sorter.hpp 68a2f56cef976203d83ed823486d1a9675f9ab68 
> 
> Diff: https://reviews.apache.org/r/49190/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message