mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 71073: Fixed /roles and GET_ROLES to expose all known roles.
Date Mon, 15 Jul 2019 21:37:44 GMT

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


Fix it, then Ship it!




Thanks for fixing this!

The indentation of the bullet points in the commit message is a bit off.


src/master/master.cpp
Lines 3531-3533 (original), 3520-3522 (patched)
<https://reviews.apache.org/r/71073/#comment303830>

    It should be more efficient to use unordered_set and then sort afterward.
    
    Not sure how much difference would it make, given the function is already expensive.



src/master/master.cpp
Lines 3537 (patched)
<https://reviews.apache.org/r/71073/#comment303831>

    not needed here?



src/master/master.cpp
Line 3538 (original), 3540 (patched)
<https://reviews.apache.org/r/71073/#comment303832>

    s/tree/set or list/ ?



src/master/master.cpp
Lines 3555-3557 (original), 3570-3577 (patched)
<https://reviews.apache.org/r/71073/#comment303829>

    can be simplified to:
    
    ```
    return vector<string>(roleList.begin(), roleList.end());
    ```


- Meng Zhu


On July 15, 2019, 1:05 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71073/
> -----------------------------------------------------------
> 
> (Updated July 15, 2019, 1:05 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9888 and MESOS-9890
>     https://issues.apache.org/jira/browse/MESOS-9888
>     https://issues.apache.org/jira/browse/MESOS-9890
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, per MESOS-9888 and MESOS-9890, the /roles and GET_ROLES
> APIs only exposed roles that had frameworks associated with them
> (either because the framework is subscribed to the role, or there
> is a framework with allocations to the role) or configured weight
> and/or quota.
> 
> This approach omits some important cases:
> 
>   (1) Roles that have only reservations associated with them.
>   (2) Roles that have only a parent relationship to other roles.
> 
> This patch exposes a function that returns all "known" roles based
> on the criteria we care about:
> 
>   (1) Roles with configured weight or quota.
>   (2) Roles with reservations.
>   (3) Roles with frameworks subscribed or allocated resources.
>   (4) Ancestor roles of (1), (2), or (3).
> 
> Also, the resource breakdowns are pulled out from the Role struct
> and placed in a function that returns the breakdowns for all known
> roles. This was done because there is currently not a Role struct
> entry for all known roles.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp cd0f40cb7b966d6620e3fb49d4c08807185c9101 
>   src/master/master.hpp e8def83fe9bcee19772df9a9764852bc694c5247 
>   src/master/master.cpp 5247377c2e7e92b9843dd4c9d28f92ba679ad742 
>   src/master/readonly_handler.cpp f4432a54a21134192636b76a5c36d0241e10dabc 
> 
> 
> Diff: https://reviews.apache.org/r/71073/diff/1/
> 
> 
> Testing
> -------
> 
> Updated a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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