You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2015/08/31 18:48:49 UTC

Review Request 37958: Add an option to only perform batch allocations.

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

Review request for mesos.


Repository: mesos


Description
-------

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs
-----

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 

Diff: https://reviews.apache.org/r/37958/diff/


Testing
-------

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.


Thanks,

James Peach


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.

> On Sept. 1, 2015, 12:18 a.m., Qian Zhang wrote:
> > include/mesos/master/allocator.hpp, line 48
> > <https://reviews.apache.org/r/37958/diff/1/?file=1060349#file1060349line48>
> >
> >     I think we do not want "{}" in the newline.

Fixed.


> On Sept. 1, 2015, 12:18 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 398
> > <https://reviews.apache.org/r/37958/diff/1/?file=1060351#file1060351line398>
> >
> >     Tab here? If so, we may need to replace tab with spaces.

AFAICT these are spaces.


> On Sept. 1, 2015, 12:18 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 397
> > <https://reviews.apache.org/r/37958/diff/1/?file=1060351#file1060351line397>
> >
> >     Instead of checking allocationOptions.allocateOnEvents everywhere, can we just do it once internally in allocate()?

The problem with doing it in allocate() is that allocate() doesn't know whether it is being called in response to a cluster event, or by the periodic allocation cycle. We could pass down a flag or wrap it in some other way, but that did not seem any less clumsy.


- James


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


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97210
-----------------------------------------------------------



include/mesos/master/allocator.hpp (line 48)
<https://reviews.apache.org/r/37958/#comment153004>

    I think we do not want "{}" in the newline.



src/master/allocator/mesos/hierarchical.hpp (line 395)
<https://reviews.apache.org/r/37958/#comment153006>

    Instead of checking allocationOptions.allocateOnEvents everywhere, can we just do it once internally in allocate()?



src/master/allocator/mesos/hierarchical.hpp (line 396)
<https://reviews.apache.org/r/37958/#comment153005>

    Tab here? If so, we may need to replace tab with spaces.


- Qian Zhang


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.

> On Sept. 1, 2015, 2:01 a.m., Guangya Liu wrote:
> > docs/configuration.md, line 292
> > <https://reviews.apache.org/r/37958/diff/1/?file=1060348#file1060348line292>
> >
> >     The following format may be better
> >     <td>
> >           If 'true' xxx (default: true)
> >           <p/>
> >           If 'false' xxx 
> >         </td>

I don't think that the `true`/`false` language makes this easier to read. I did update the language slightly and I hope that it clarifies the behaviour.


- James


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


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97227
-----------------------------------------------------------



docs/configuration.md (line 288)
<https://reviews.apache.org/r/37958/#comment153043>

    is it only a flag or also have value? If only flag, can we use the following:
    --[no-]allocate_on_events



docs/configuration.md (line 292)
<https://reviews.apache.org/r/37958/#comment153044>

    The following format may be better
    <td>
          If 'true' xxx (default: true)
          <p/>
          If 'false' xxx 
        </td>


- Guangya Liu


On 八月 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated 八月 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97290
-----------------------------------------------------------

Ship it!


LGTM. I would love us to avoid creating a master flag for each allocator option, mind creating a JIRA for this?

- Alexander Rukletsov


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

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


Patch looks great!

Reviews applied: [37958]

All tests passed.

- Mesos ReviewBot


On Sept. 1, 2015, 4:34 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97596
-----------------------------------------------------------


Hey James, I left some comments on [MESOS-3157](https://issues.apache.org/jira/browse/MESOS-3157), please have a look!

- Ben Mahler


On Sept. 1, 2015, 11:10 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 11:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97370
-----------------------------------------------------------

Ship it!


Ship It!

- Guangya Liu


On 九月 1, 2015, 11:10 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated 九月 1, 2015, 11:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

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


Patch looks great!

Reviews applied: [37958]

All tests passed.

- Mesos ReviewBot


On Sept. 1, 2015, 11:10 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 11:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/
-----------------------------------------------------------

(Updated Sept. 1, 2015, 11:10 p.m.)


Review request for mesos and Ben Mahler.


Changes
-------

Updated with bmahler's review comments.


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


Repository: mesos


Description
-------

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs (updated)
-----

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

Diff: https://reviews.apache.org/r/37958/diff/


Testing
-------

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.


Thanks,

James Peach


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.

> On Sept. 1, 2015, 6 p.m., Ben Mahler wrote:
> > include/mesos/master/allocator.hpp, lines 43-56
> > <https://reviews.apache.org/r/37958/diff/2/?file=1061153#file1061153line43>
> >
> >     How about a 'struct' with no default constructor? Putting in these default values is a bit odd, since these fields should be coming from the master flags.

This is needed for uses that don't get flags from the master (eg. tests).


- James


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


On Sept. 1, 2015, 4:34 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.

> On Sept. 1, 2015, 6 p.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 284
> > <https://reviews.apache.org/r/37958/diff/2/?file=1061155#file1061155line284>
> >
> >     Can you move this back up to below 'initialized' for now?

Fixed.


> On Sept. 1, 2015, 6 p.m., Ben Mahler wrote:
> > include/mesos/master/allocator.hpp, line 81
> > <https://reviews.apache.org/r/37958/diff/2/?file=1061153#file1061153line81>
> >
> >     How about s/allocationOptions/options/ here and everywhere else in this patch?

Renamed to ```allocator::Options```.


> On Sept. 1, 2015, 6 p.m., Ben Mahler wrote:
> > docs/configuration.md, lines 291-295
> > <https://reviews.apache.org/r/37958/diff/2/?file=1061152#file1061152line291>
> >
> >     Can you have this match the help message from flags.cpp? In particular, let's not tell people that this gives more predictable performance, as that is alluding to the fact that there is are performance bug(s) in the allocator, and we should fix those!

Done.


- James


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


On Sept. 1, 2015, 4:34 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/#review97311
-----------------------------------------------------------

Ship it!


Just some minor updates and we can get this committed. Thanks James!

Let's be clear in the description that this is motivated by some performance bugs in which event-based allocation will enqueue too many allocation rounds, and the allocator's performance is degraded due to the allocator's queue getting backed up. "Unpredictable performance" might not be clear to those who encounter this later :)


docs/configuration.md (lines 291 - 295)
<https://reviews.apache.org/r/37958/#comment153150>

    Can you have this match the help message from flags.cpp? In particular, let's not tell people that this gives more predictable performance, as that is alluding to the fact that there is are performance bug(s) in the allocator, and we should fix those!



include/mesos/master/allocator.hpp (lines 43 - 56)
<https://reviews.apache.org/r/37958/#comment153151>

    How about a 'struct' with no default constructor? Putting in these default values is a bit odd, since these fields should be coming from the master flags.



include/mesos/master/allocator.hpp (line 81)
<https://reviews.apache.org/r/37958/#comment153152>

    How about s/allocationOptions/options/ here and everywhere else in this patch?



src/master/allocator/mesos/hierarchical.hpp (line 282)
<https://reviews.apache.org/r/37958/#comment153153>

    Can you move this back up to below 'initialized' for now?


- Ben Mahler


On Sept. 1, 2015, 4:34 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/
-----------------------------------------------------------

(Updated Sept. 1, 2015, 4:34 p.m.)


Review request for mesos and Ben Mahler.


Changes
-------

Rebased onto master and addressed some review comments.


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


Repository: mesos


Description
-------

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs (updated)
-----

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

Diff: https://reviews.apache.org/r/37958/diff/


Testing
-------

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.


Thanks,

James Peach


Re: Review Request 37958: Add an option to only perform batch allocations.

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


Patch looks great!

Reviews applied: [37958]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
>     https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> -------
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 37958: Add an option to only perform batch allocations.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37958/
-----------------------------------------------------------

(Updated Aug. 31, 2015, 4:49 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
-------

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs
-----

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 

Diff: https://reviews.apache.org/r/37958/diff/


Testing
-------

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple weeks.


Thanks,

James Peach