You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/07/19 21:32:46 UTC

Review Request 71130: Added a test to ensure allocations are restricted by quota limits.

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
-------

Added a test to ensure allocations are restricted by quota limits.


Diffs
-----

  src/tests/hierarchical_allocator_tests.cpp 9ebeeb6501e909544f5461710059ded66fb70eed 


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


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 71130: Added a test to ensure allocations are restricted by quota limits.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71130/#review216803
-----------------------------------------------------------


Fix it, then Ship it!





src/tests/hierarchical_allocator_tests.cpp
Lines 3573-3575 (patched)
<https://reviews.apache.org/r/71130/#comment304026>

    There needs to be a settle before this to make sure the addSlave call completed executing?
    
    As an aside, it would be nice if we surfaced the Futures from these calls (e.g. addSlave) rather than hiding them with a void return type. Then we could just AWAIT_READY here instead of settling.



src/tests/hierarchical_allocator_tests.cpp
Lines 3588-3589 (patched)
<https://reviews.apache.org/r/71130/#comment304027>

    "Now raise the limit from 0 to ... which should make the role get resources up to this limit" ?



src/tests/hierarchical_allocator_tests.cpp
Lines 3627-3628 (patched)
<https://reviews.apache.org/r/71130/#comment304028>

    use `*` operator instead of .get() here and above?


- Benjamin Mahler


On July 19, 2019, 9:32 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71130/
> -----------------------------------------------------------
> 
> (Updated July 19, 2019, 9:32 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a test to ensure allocations are restricted by quota limits.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp 9ebeeb6501e909544f5461710059ded66fb70eed 
> 
> 
> Diff: https://reviews.apache.org/r/71130/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71130: Added a test to ensure allocations are restricted by quota limits.

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



Patch looks great!

Reviews applied: [71131, 71130]

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 July 19, 2019, 9:32 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71130/
> -----------------------------------------------------------
> 
> (Updated July 19, 2019, 9:32 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a test to ensure allocations are restricted by quota limits.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp 9ebeeb6501e909544f5461710059ded66fb70eed 
> 
> 
> Diff: https://reviews.apache.org/r/71130/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>