mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request 37177: Maintenance Primitives: Added inverse offers.
Date Fri, 28 Aug 2015 01:58:11 GMT

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

Ship it!


Again, with the rebase changes from earlier reviews capturing "maintenance" specific bits.


include/mesos/maintenance/maintenance.hpp (line 44)
<https://reviews.apache.org/r/37177/#comment152499>

    ... between the Allocator and Master in order to let the Master create InverseOffers from
the Allocator.



include/mesos/master/allocator.hpp (line 29)
<https://reviews.apache.org/r/37177/#comment152500>

    Newline please.



src/master/allocator/mesos/hierarchical.hpp (line 183)
<https://reviews.apache.org/r/37177/#comment152494>

    s/inverse/deallocate/



src/master/allocator/mesos/hierarchical.hpp (line 1108)
<https://reviews.apache.org/r/37177/#comment152495>

    Let's comment that for now we "implement maintenance inverse offers" by leveraging the
existing allocation timer/cycle to also do any "deallocations" necessary (via inverse offers)
to satisfy the maintenance needs.



src/master/allocator/mesos/hierarchical.hpp (line 1122)
<https://reviews.apache.org/r/37177/#comment152496>

    How about a comment explaining that what we deem to be "offerable" is actually what we
want the master to create InverseOffers from ...



src/master/master.cpp (line 4777)
<https://reviews.apache.org/r/37177/#comment152493>

    Indentation.



src/tests/hierarchical_allocator_tests.cpp (line 82)
<https://reviews.apache.org/r/37177/#comment152498>

    s/Revocation/Deallocation/



src/tests/hierarchical_allocator_tests.cpp (lines 126 - 127)
<https://reviews.apache.org/r/37177/#comment152497>

    Why not use C++11 lambdas here? I get that it will be a verbose lambda because the parameters
are verbose, but still seems like the better approach.


- Benjamin Hindman


On Aug. 26, 2015, 2:12 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e

>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf

>   src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
> 
> Diff: https://reviews.apache.org/r/37177/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


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