You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/08/10 09:05:29 UTC
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Aug. 10, 2015, 7:05 a.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
Summary (updated)
-----------------
Maintenance Primitives: Added inverse offers.
Bugs: MESOS-1474
https://issues.apache.org/jira/browse/MESOS-1474
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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 e278139f856888d6c6f538f7c0f664087e97f629
src/master/master.hpp 28356e4ca24312b8be0138a34805b3d9035a99a3
src/master/master.cpp 08dd34d9d18f547c6e8d04caf9e39a2b3ffc5f63
src/tests/hierarchical_allocator_tests.cpp c92d47aa0a06088f1f8fe749a629c27bfadd3c6d
src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2
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
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review95805
-----------------------------------------------------------
include/mesos/maintenance/maintenance.hpp (line 38)
<https://reviews.apache.org/r/37177/#comment150949>
Remove this blank space. The commit should not be permitted if there are blank space in code.
- Guangya Liu
On 八月 18, 2015, 6:57 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated 八月 18, 2015, 6:57 p.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 e278139f856888d6c6f538f7c0f664087e97f629
> src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c
> 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
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review96375
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 2:13 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 e278139f856888d6c6f538f7c0f664087e97f629
> src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
> 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
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review98186
-----------------------------------------------------------
Ship it!
Ship It!
- Yong Qiao Wang
On Sept. 2, 2015, 7:32 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 7:32 p.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 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
> include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
> src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
> src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e
> src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569
> src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
> src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
>
> Diff: https://reviews.apache.org/r/37177/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review98337
-----------------------------------------------------------
Ship it!
include/mesos/maintenance/maintenance.hpp (line 31)
<https://reviews.apache.org/r/37177/#comment154748>
s/Master/Allocator/
src/master/allocator/mesos/hierarchical.hpp (line 1119)
<https://reviews.apache.org/r/37177/#comment154756>
Can we use NOTE: here like in the other review? Thanks!
src/master/allocator/mesos/hierarchical.hpp (line 1155)
<https://reviews.apache.org/r/37177/#comment154751>
A similar suggestion to a previous review:
Option<MaintenanceStatus> maintenanceStatus = slaves[slaveId].maintenanceStatus;
if (maintenanceStatus.isSome()) {
...;
if (!maintenanceStatus->statuses.contains(frameworkId) || ...) {
...;
}
...;
}
Or '!maintenanceStatus.get().statuses.contains(frameworkId)' if you prefer the '.get()' variant (ahhh! this is what I was worried about!).
src/tests/hierarchical_allocator_tests.cpp (lines 195 - 196)
<https://reviews.apache.org/r/37177/#comment154750>
Can we please s/offerQueue/allocations/ s/inverseOfferQueue/deallocations/ thanks!
- Benjamin Hindman
On Sept. 2, 2015, 7:32 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 7:32 p.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 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
> include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
> src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
> src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e
> src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569
> src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
> src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
>
> Diff: https://reviews.apache.org/r/37177/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Sept. 14, 2015, 12:51 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
Changes
-------
rebased.
Bugs: MESOS-1474
https://issues.apache.org/jira/browse/MESOS-1474
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/maintenance/maintenance.hpp 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
src/master/allocator/mesos/hierarchical.hpp fbf353d8bdd4322275057e392a958fca77ecd8b3
src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24
src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574
src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
src/tests/mesos.hpp 6b5031191b81a18f9596f547a1e0f67e35881cc3
src/tests/reservation_endpoints_tests.cpp dfab4971e04a81ac98ed118ea877bcca5db17bb5
src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5
Diff: https://reviews.apache.org/r/37177/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review98801
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Sept. 13, 2015, 8:33 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2015, 8:33 p.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 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
> include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
> src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
> src/master/allocator/mesos/hierarchical.hpp fbf353d8bdd4322275057e392a958fca77ecd8b3
> src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24
> src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 6b5031191b81a18f9596f547a1e0f67e35881cc3
> src/tests/reservation_endpoints_tests.cpp dfab4971e04a81ac98ed118ea877bcca5db17bb5
> src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
> src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
> src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5
>
> Diff: https://reviews.apache.org/r/37177/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Sept. 13, 2015, 8:33 p.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 (updated)
-----
include/mesos/maintenance/maintenance.hpp 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
src/master/allocator/mesos/hierarchical.hpp fbf353d8bdd4322275057e392a958fca77ecd8b3
src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24
src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574
src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
src/tests/mesos.hpp 6b5031191b81a18f9596f547a1e0f67e35881cc3
src/tests/reservation_endpoints_tests.cpp dfab4971e04a81ac98ed118ea877bcca5db17bb5
src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5
Diff: https://reviews.apache.org/r/37177/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review97529
-----------------------------------------------------------
Ship it!
LGTM!
src/master/allocator/mesos/hierarchical.hpp (line 1119)
<https://reviews.apache.org/r/37177/#comment153496>
Nano-nit: You used `NOTE:` on all the other "we implement maintenance here" comments.
- Joseph Wu
On Sept. 2, 2015, 12:32 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 12:32 p.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 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
> include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
> src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
> src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e
> src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569
> src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
> src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
>
> Diff: https://reviews.apache.org/r/37177/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Sept. 2, 2015, 7:32 p.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 (updated)
-----
include/mesos/maintenance/maintenance.hpp 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e
src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569
src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
Diff: https://reviews.apache.org/r/37177/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
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
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Aug. 31, 2015, 1:56 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 1118
> > <https://reviews.apache.org/r/37177/diff/5/?file=1052923#file1052923line1118>
> >
> > s/send inverse offer/deallocate resources/
This is tricky. We decided to use the word "de-allocate" in the allocator code. At the same time, we're still using the "inverse offer" terminology elsewhere (in the master and scheduler API), as well as in presentations and documentation. Considering the latter, I think it is more usefull for users to see error messages using the "inverse offer" terminology.
> On Aug. 31, 2015, 1:56 p.m., Qian Zhang wrote:
> > src/master/master.cpp, line 623
> > <https://reviews.apache.org/r/37177/diff/5/?file=1052925#file1052925line623>
> >
> > In future, we may need to add more callback functions, so here can we change to use a general structure to keep the callback functions that we want to pass into allocator? So in future, if we want to add more callback functions, just need to change that structure rather than the signature of allocator->initialize()
I understand your reasoning here. In this case I prefer making it "harder" on a developer to modify the API, as the C++ compiler will guide them through all the consequences of changing the API. If we turned this into a dynamic change of behavior, then it would increase the risk of bugs or half-implemented features.
Regardless, this API will be getting some love (refactoring) in the near future, where some of your concerns will be addressed :-)
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review97073
-----------------------------------------------------------
On Sept. 13, 2015, 8:33 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2015, 8:33 p.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 7fec3ffe063e766dcec4952001fa5c6e20d9eec8
> include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a
> src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659
> src/master/allocator/mesos/hierarchical.hpp fbf353d8bdd4322275057e392a958fca77ecd8b3
> src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24
> src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 6b5031191b81a18f9596f547a1e0f67e35881cc3
> src/tests/reservation_endpoints_tests.cpp dfab4971e04a81ac98ed118ea877bcca5db17bb5
> src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b
> src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519
> src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5
>
> Diff: https://reviews.apache.org/r/37177/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review97073
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp (line 391)
<https://reviews.apache.org/r/37177/#comment152804>
Indent:
Line 392: Need to delete a space.
Line 393: Need to add 7 more spaces to align with line 392.
src/master/allocator/mesos/hierarchical.hpp (line 1118)
<https://reviews.apache.org/r/37177/#comment152807>
s/send inverse offer/deallocate resources/
src/master/allocator/mesos/hierarchical.hpp (line 1136)
<https://reviews.apache.org/r/37177/#comment152809>
So here we only use framework sorters, then why we check role sorter in the beginning of this method?
src/master/allocator/mesos/hierarchical.hpp (line 1154)
<https://reviews.apache.org/r/37177/#comment152811>
Remove "we"?
src/master/master.cpp (line 623)
<https://reviews.apache.org/r/37177/#comment152803>
In future, we may need to add more callback functions, so here can we change to use a general structure to keep the callback functions that we want to pass into allocator? So in future, if we want to add more callback functions, just need to change that structure rather than the signature of allocator->initialize()
- Qian Zhang
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
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----
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
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Aug. 25, 2015, 2:13 a.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
Changes
-------
Rebased.
Bugs: MESOS-1474
https://issues.apache.org/jira/browse/MESOS-1474
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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 e278139f856888d6c6f538f7c0f664087e97f629
src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
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
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review95785
-----------------------------------------------------------
Ship it!
Ship It!
- Joseph Wu
On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2015, 11:57 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 e278139f856888d6c6f538f7c0f664087e97f629
> src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
> src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
> src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c
> 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
>
>
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/
-----------------------------------------------------------
(Updated Aug. 18, 2015, 6:57 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
Changes
-------
Rebased.
Bugs: MESOS-1474
https://issues.apache.org/jira/browse/MESOS-1474
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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 e278139f856888d6c6f538f7c0f664087e97f629
src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
src/tests/hierarchical_allocator_tests.cpp 9748ca0b38888fee25dcec51c64d8ba84dbd4aaf
src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c
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
Re: Review Request 37177: Maintenance Primitives: Added inverse
offers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37177/#review95165
-----------------------------------------------------------
include/mesos/maintenance/maintenance.hpp (line 33)
<https://reviews.apache.org/r/37177/#comment150007>
It seems like we don't need this field yet. Is it just included so that we don't change the public headers more than once?
src/master/allocator/mesos/hierarchical.hpp (line 1151)
<https://reviews.apache.org/r/37177/#comment150005>
Isn't `offerable[frameworkId]` always undefined?
- Joseph Wu
On Aug. 10, 2015, 12:05 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2015, 12:05 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 e278139f856888d6c6f538f7c0f664087e97f629
> src/master/master.hpp 28356e4ca24312b8be0138a34805b3d9035a99a3
> src/master/master.cpp 08dd34d9d18f547c6e8d04caf9e39a2b3ffc5f63
> src/tests/hierarchical_allocator_tests.cpp c92d47aa0a06088f1f8fe749a629c27bfadd3c6d
> src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24
> src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2
> 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
>
>