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 71255: Fixed and improved `Sorter::remove` function.
Date Tue, 13 Aug 2019 20:46:35 GMT

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


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