----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71255/#review217185 -----------------------------------------------------------
Ship it! src/master/allocator/mesos/sorter/drf/sorter.cpp Line 212 (original), 213 (patched) <https://reviews.apache.org/r/71255/#comment304472> The whole logic here seems to be dealing with the single "." child case. Maybe simpler to grasp as a single condition and with less nesting? ``` } else if (current->children.size() == 1 && current->children[0]->name == ".") { ... } ``` src/master/allocator/mesos/sorter/random/sorter.cpp Line 206 (original), 207 (patched) <https://reviews.apache.org/r/71255/#comment304473> Ditto here. - Benjamin Mahler On Aug. 8, 2019, 7:34 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71255/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2019, 7:34 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-9930 > https://issues.apache.org/jira/browse/MESOS-9930 > > > Repository: mesos > > > Description > ------- > > There is a bug in the remove() function where after collapsing > and turning an internal node into an leaf node, the new node's > position needs to updated in the parent's children list. > See MESOS-9930. > > This patch fixes the bug and also refactored the function logic. > > > Diffs > ----- > > src/master/allocator/mesos/sorter/drf/sorter.cpp > 40397f7111d92dccecd15c9bc1ad7c45abbd850b > src/master/allocator/mesos/sorter/random/sorter.cpp > b480aeee38f87dca852ad3c8f97bb4fe993ce068 > > > Diff: https://reviews.apache.org/r/71255/diff/1/ > > > Testing > ------- > > make check > > dedicated test added in r/71256/ > > > Thanks, > > Meng Zhu > >