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 70968: Fixed adding and removing framework roles in suppressed state.
Date Thu, 27 Jun 2019 23:55:17 GMT

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



Thanks for fixing this! Fixed logic looks good, but the commit description is vague. Can you
expand the commit description to clarify what the issues were and what this patch does to
fix them? (example below)


src/master/allocator/mesos/hierarchical.cpp
Lines 552 (patched)
<https://reviews.apache.org/r/70968/#comment303320>

    Per the top-level comment, can you document in the commit description each issue that
was fixed?
    
    This one looks like:
    
    -When a role is removed, if the framework still has resources allocated to the role we
previously did not deactivate the role. (... what's the consequence of this as well?)



src/master/allocator/mesos/hierarchical.cpp
Lines 550-551 (original), 568-569 (patched)
<https://reviews.apache.org/r/70968/#comment303319>

    Per the comment in the previous review, it's suprising that newUnsuppressedRoles can contain
entries in removedRoles, shall we just adjust the set construction so that it does not?


- Benjamin Mahler


On June 27, 2019, 7:38 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70968/
> -----------------------------------------------------------
> 
> (Updated June 27, 2019, 7:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9870
>     https://issues.apache.org/jira/browse/MESOS-9870
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch fixes bugs in updateFramework() in the allocator code
> which can be triggered by UPDATE_FRAMEWORK or re-subscription -
> see MESOS-9870.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 26aad6778f12b99bb87c846788d6b6d60f743d8a

> 
> 
> Diff: https://reviews.apache.org/r/70968/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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