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 71073: Fixed /roles and GET_ROLES to expose all known roles.
Date Mon, 15 Jul 2019 22:27:59 GMT


> On July 15, 2019, 9:37 p.m., Meng Zhu wrote:
> > Thanks for fixing this!
> > 
> > The indentation of the bullet points in the commit message is a bit off.

Seems to be reviewboard's rendering.


> On July 15, 2019, 9:37 p.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Lines 3531-3533 (original), 3520-3522 (patched)
> > <https://reviews.apache.org/r/71073/diff/1/?file=2154910#file2154910line3531>
> >
> >     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.

Will it? It doesn't seem that obvious to me that it will be faster, but I can't easily measure
now.


> On July 15, 2019, 9:37 p.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Lines 3537 (patched)
> > <https://reviews.apache.org/r/71073/diff/1/?file=2154910#file2154910line3548>
> >
> >     not needed here?

As discussed offline, removing this would make it inconsistent about when ancestors are exposed.
E.g. you wouldn't be able to necessarily see "eng" when "eng/frontend" and "eng/backend" entries
are present, which seems unfortunate for the user and for the UI to try to deal with.


- Benjamin


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


On July 15, 2019, 8: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, 8: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