mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 70967: Improved readability of updateFramework() in the hierarchial allocator.
Date Sat, 29 Jun 2019 13:00:27 GMT


> On June 27, 2019, 11:59 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 488-505 (original), 488-522 (patched)
> > <https://reviews.apache.org/r/70967/diff/1/?file=2152626#file2152626line488>
> >
> >     Have you considered implementing the TODO to tidy this up?
> >     
> >     ```
> >     const set<string> removedRoles = oldRoles - newRoles;
> >     const set<string> addedRoles = newRoles - oldRoles;
> >     
> >     const set<string> newSuppressedRoles = suppressedRoles - oldSuppressedRoles;
> >     const set<string> oldSuppressedRoles = oldSuppressedRoles - suppressedRoles;
> >     ```
> >     
> >     Needs adding a function here that uses std::set_difference:
> >     
> >     https://github.com/apache/mesos/blob/1.8.0/3rdparty/stout/include/stout/set.hpp
> >     
> >     Of course, feel free to do this after the fix.

After looking at this all once again, I opted for implementing this TODO - makes the bug fixes
much more transparent.


- Andrei


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


On June 27, 2019, 7:37 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70967/
> -----------------------------------------------------------
> 
> (Updated June 27, 2019, 7:37 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 moves the state-manipulating bits closer together, replaces
> the misleading NewRevivedRoles name with NewUnsuppressedRoles and adds
> const qualifiers to locals.
> 
> This is a prerequisite to fix of MESOS-9870 bug in the next patch.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 26aad6778f12b99bb87c846788d6b6d60f743d8a

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


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