You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2019/10/24 17:07:42 UTC

Review Request 71673: Got rid of storing totals in the random sorter.

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

Review request for mesos, Benjamin Mahler and Meng Zhu.


Bugs: MESOS-10015
    https://issues.apache.org/jira/browse/MESOS-10015


Repository: mesos


Description
-------

Got rid of storing totals in the random sorter.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 9d0fbe771868ea60e66b9e25b0c666d5416d6e85 
  src/master/allocator/mesos/hierarchical.cpp 21010de363f25c516bb031e4ae48888e53621128 
  src/master/allocator/mesos/sorter/drf/sorter.hpp 3f6c7413f1b76f3fa86388360983763c8b76079f 
  src/master/allocator/mesos/sorter/drf/sorter.cpp ef79083b710fba628b4a7e93f903883899f8a71b 
  src/master/allocator/mesos/sorter/random/sorter.hpp a3097be98d175d2b47714eb8b70b1ce8c5c2bba8 
  src/master/allocator/mesos/sorter/random/sorter.cpp 86aeb1b8136eaffd2d52d3b603636b01383a9024 
  src/master/allocator/mesos/sorter/sorter.hpp 6b6b4a1811ba36e0212de17b9a6e63a6f8678a7f 


Diff: https://reviews.apache.org/r/71673/diff/1/


Testing
-------

make check


Thanks,

Andrei Sekretenko


Re: Review Request 71673: Got rid of storing totals in the random sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71673/#review218402
-----------------------------------------------------------



Patch looks great!

Reviews applied: [71646, 71672, 71673]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 24, 2019, 5:07 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71673/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2019, 5:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-10015
>     https://issues.apache.org/jira/browse/MESOS-10015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Got rid of storing totals in the random sorter.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 9d0fbe771868ea60e66b9e25b0c666d5416d6e85 
>   src/master/allocator/mesos/hierarchical.cpp 21010de363f25c516bb031e4ae48888e53621128 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 3f6c7413f1b76f3fa86388360983763c8b76079f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp ef79083b710fba628b4a7e93f903883899f8a71b 
>   src/master/allocator/mesos/sorter/random/sorter.hpp a3097be98d175d2b47714eb8b70b1ce8c5c2bba8 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 86aeb1b8136eaffd2d52d3b603636b01383a9024 
>   src/master/allocator/mesos/sorter/sorter.hpp 6b6b4a1811ba36e0212de17b9a6e63a6f8678a7f 
> 
> 
> Diff: https://reviews.apache.org/r/71673/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 71673: Got rid of storing totals in the random sorter.

Posted by Andrei Sekretenko <as...@mesosphere.io>.

> On Oct. 25, 2019, 12:13 a.m., Meng Zhu wrote:
> > src/master/allocator/mesos/sorter/sorter.hpp
> > Line 130 (original), 127 (patched)
> > <https://reviews.apache.org/r/71673/diff/1/?file=2170231#file2170231line130>
> >
> >     Don't think we need to be 100% accurate here, the except part would only lead to confusion. Maybe add a "may" or just leave it as-is.

Changed 'will' into 'may'.


- Andrei


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


On Oct. 24, 2019, 5:07 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71673/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2019, 5:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-10015
>     https://issues.apache.org/jira/browse/MESOS-10015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Got rid of storing totals in the random sorter.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 9d0fbe771868ea60e66b9e25b0c666d5416d6e85 
>   src/master/allocator/mesos/hierarchical.cpp 21010de363f25c516bb031e4ae48888e53621128 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 3f6c7413f1b76f3fa86388360983763c8b76079f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp ef79083b710fba628b4a7e93f903883899f8a71b 
>   src/master/allocator/mesos/sorter/random/sorter.hpp a3097be98d175d2b47714eb8b70b1ce8c5c2bba8 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 86aeb1b8136eaffd2d52d3b603636b01383a9024 
>   src/master/allocator/mesos/sorter/sorter.hpp 6b6b4a1811ba36e0212de17b9a6e63a6f8678a7f 
> 
> 
> Diff: https://reviews.apache.org/r/71673/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 71673: Got rid of storing totals in the random sorter.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71673/#review218404
-----------------------------------------------------------


Fix it, then Ship it!





src/master/allocator/mesos/sorter/random/sorter.hpp
Lines 101-103 (original), 100-102 (patched)
<https://reviews.apache.org/r/71673/#comment306116>

    hmm, I thought this should be
    ```
    override {}
    ```



src/master/allocator/mesos/sorter/sorter.hpp
Line 130 (original), 127 (patched)
<https://reviews.apache.org/r/71673/#comment306115>

    Don't think we need to be 100% accurate here, the except part would only lead to confusion. Maybe add a "may" or just leave it as-is.


- Meng Zhu


On Oct. 24, 2019, 10:07 a.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71673/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2019, 10:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-10015
>     https://issues.apache.org/jira/browse/MESOS-10015
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Got rid of storing totals in the random sorter.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 9d0fbe771868ea60e66b9e25b0c666d5416d6e85 
>   src/master/allocator/mesos/hierarchical.cpp 21010de363f25c516bb031e4ae48888e53621128 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 3f6c7413f1b76f3fa86388360983763c8b76079f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp ef79083b710fba628b4a7e93f903883899f8a71b 
>   src/master/allocator/mesos/sorter/random/sorter.hpp a3097be98d175d2b47714eb8b70b1ce8c5c2bba8 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 86aeb1b8136eaffd2d52d3b603636b01383a9024 
>   src/master/allocator/mesos/sorter/sorter.hpp 6b6b4a1811ba36e0212de17b9a6e63a6f8678a7f 
> 
> 
> Diff: https://reviews.apache.org/r/71673/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>