> 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 > >
