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 70707: Refactored role quota info into `Struct Role`.
Date Fri, 24 May 2019 23:57:57 GMT

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




src/master/allocator/mesos/hierarchical.hpp
Line 123 (original), 125-126 (patched)
<https://reviews.apache.org/r/70707/#comment302252>

    just one on each line at this point seems more readable?



src/master/allocator/mesos/hierarchical.hpp
Lines 644-645 (patched)
<https://reviews.apache.org/r/70707/#comment302253>

    "Has quota" seems inaccurate and will break down soon when we introduce the ability to
set the limits.
    
    Strictly speaking, every role has a quota, it just might be 0 guarantee / no limit (default).
    
    Can't this be a property of the role struct? Looking at code like this:
    
    ```
      if (roleHasQuota(role)) {
    ```
    
    Makes me think the code should just use the role struct:
    
    ```
      Role r = roles.at(role);
      
      if (r.quota.guarantee > ResourceQuantities()) {
        ...
      }
    ```



src/master/allocator/mesos/hierarchical.cpp
Lines 2558 (patched)
<https://reviews.apache.org/r/70707/#comment302254>

    Per my above comment, when would roles not contain role when we're asking about whether
a role has quota?


- Benjamin Mahler


On May 24, 2019, 11:52 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70707/
> -----------------------------------------------------------
> 
> (Updated May 24, 2019, 11:52 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This avoids an extra map of tracking role quota info.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 7e4f30d16010a777ff29ba35366ae80ff2631322

>   src/master/allocator/mesos/hierarchical.cpp ccca391fbf73d9aa9c31f7552e39fb78c242e242

> 
> 
> Diff: https://reviews.apache.org/r/70707/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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