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 2018/03/10 00:44:57 UTC
Review Request 66016: Refactored resource allocation logic.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
Repository: mesos
Description
-------
The current allocation logic maintains several states
such as allocated-reservation, unsatisfied-quota and etc.
This patch refactors the logic so that we only need to
maintain a per-quota-role map to track role consumed quota.
Diffs
-----
src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
Diff: https://reviews.apache.org/r/66016/diff/1/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review198989
-----------------------------------------------------------
Patch looks great!
Reviews applied: [66016]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 10, 2018, 12:44 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 10, 2018, 12:44 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review199048
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['66016']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016/logs/mesos-tests-stdout.log):
```
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (126 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (1187 ms total)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (37 ms)
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (45 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (84 ms total)
[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[ OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2549 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (2573 ms total)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2570 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (2594 ms total)
[----------] Global test environment tear-down
[==========] 916 tests from 91 test cases ran. (468263 ms total)
[ PASSED ] 915 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] SlaveTest.RestartSlaveRequireExecutorAuthentication
1 FAILED TEST
YOU HAVE 210 DISABLED TESTS
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016/logs/mesos-tests-stderr.log):
```
I0313 00:14:38.126778 3456 master.cpp:10245] Updating the state of task 7585aff6-5367-47ae-8720-874fc68ad489 of framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0313 00:14:38.126778 10300 slave.cpp:3878] Shutting down framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-0000
I0313 00:14:38.127776 10300 slave.cpp:6571] Shutting down executor '7585aff6-5367-47ae-8720-874fc68ad489' of framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-I0313 00:14:37.422773 992 exec.cpp:162] Version: 1.6.0
I0313 00:14:37.450773 5208 exec.cpp:236] Executor registered on agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0
I0313 00:14:37.455775 3568 executor.cpp:176] Received SUBSCRIBED event
I0313 00:14:37.460777 3568 executor.cpp:180] Subscribed executor on build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0313 00:14:37.461776 3568 executor.cpp:176] Received LAUNCH event
I0313 00:14:37.466778 3568 executor.cpp:648] Starting task 7585aff6-5367-47ae-8720-874fc68ad489
I0313 00:14:37.548775 3568 executor.cpp:483] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I0313 00:14:38.093776 3568 executor.cpp:661] Forked command at 10460
I0313 00:14:38.131774 7948 exec.cpp:445] Executor asked to shutdown
I0313 00:14:38.131774 3568 executor.cpp:176] Received SHUTDOWN event
I0313 00:14:38.132773 3568 executor.cpp:758] Shutting down
I0313 00:14:38.132773 3568 executor.cpp:868] Sending SIGTERM to process tree at pid 0000 at executor(1)@10.3.1.5:56110
I0313 00:14:38.129777 10300 slave.cpp:924] Agent terminating
W0313 00:14:38.129777 10300 slave.cpp:3874] Ignoring shutdown framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-0000 because it is terminating
I0313 00:14:38.129777 3456 master.cpp:10344] Removing task 7585aff6-5367-47ae-8720-874fc68ad489 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-0000 on agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0 at slave(398)@10.3.1.5:56089 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0313 00:14:38.132773 5756 containerizer.cpp:2338] Destroying container cbf5b054-c117-4d63-9f04-3cc549b01d8a in RUNNING state
I0313 00:14:38.132773 5756 containerizer.cpp:2952] Transitioning the state of container cbf5b054-c117-4d63-9f04-3cc549b01d8a from RUNNING to DESTROYING
I0313 00:14:38.132773 3456 master.cpp:1288] Agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0 at slave(398)@10.3.1.5:56089 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0313 00:14:38.133774 8204 hierarchical.cpp:344] Removed framework 32e127f4-6416-43b0-8dda-e21df6d56b0c-0000
I0313 00:14:38.133774 5756 launcher.cpp:156] Asked to destroy container cbf5b054-c117-4d63-9f04-3cc549b01d8a
I0313 00:14:38.133774 3456 master.cpp:3258] Disconnecting agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0 at slave(398)@10.3.1.5:56089 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0313 00:14:38.134774 3456 master.cpp:3277] Deactivating agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0 at slave(398)@10.3.1.5:56089 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0313 00:14:38.134774 7600 hierarchical.cpp:766] Agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0 deactivated
I0313 00:14:38.160861 5756 containerizer.cpp:2791] Container cbf5b054-c117-4d63-9f04-3cc549b01d8a has exited
I0313 00:14:38.195858 2748 master.cpp:1131] Master terminating
I0313 00:14:38.199834 3456 hierarchical.cpp:609] Removed agent 32e127f4-6416-43b0-8dda-e21df6d56b0c-S0
I0313 00:14:38.684109 8968 process.cpp:929] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On March 12, 2018, 4:07 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 12, 2018, 4:07 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review199047
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Mahler
On March 12, 2018, 11:07 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 12, 2018, 11:07 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/
-----------------------------------------------------------
(Updated March 12, 2018, 4:07 p.m.)
Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
Changes
-------
Thanks for the review!
Repository: mesos
Description
-------
The current allocation logic maintains several states
such as allocated-reservation, unsatisfied-quota and etc.
This patch refactors the logic so that we only need to
maintain a per-quota-role map to track role consumed quota.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
Diff: https://reviews.apache.org/r/66016/diff/3/
Changes: https://reviews.apache.org/r/66016/diff/2-3/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review198984
-----------------------------------------------------------
PASS: Mesos patch 66016 was successfully built and tested.
Reviews applied: `['66016']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016
- Mesos Reviewbot Windows
On March 10, 2018, 12:44 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 10, 2018, 12:44 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review198979
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['66016']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016/logs/mesos-tests-stdout.log):
```
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (130 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (1203 ms total)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (40 ms)
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (44 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (86 ms total)
[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[ OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2547 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (2571 ms total)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2452 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (2477 ms total)
[----------] Global test environment tear-down
[==========] 916 tests from 91 test cases ran. (472525 ms total)
[ PASSED ] 915 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] CommandExecutorCheckTest.CommandCheckTimeout
1 FAILED TEST
YOU HAVE 210 DISABLED TESTS
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66016/logs/mesos-tests-stderr.log):
```
I0310 01:49:30.325629 10884 slave.cpp:3878] Shutting down framework fd8c9e29-7d14-4533-bb86-fac8e2084c51-0000
I0310 01:49:30.325629 4588 master.cpp:10214] Updating the state of task 54c49790-38d0-4357-8934-e1f431b5f286 of framework fd8c9e29-7d14-4533-bb86-fac8e2084c51-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0310 01:49:30.325629 10884 slave.cpp:6571] Shutting down executor '54c49790-38d0-4357-8934-e1f431b5f286' of framework fd8c9e29-7d14-4533-bb8I0310 01:49:29.616658 8928 exec.cpp:162] Version: 1.6.0
I0310 01:49:29.646654 6484 exec.cpp:236] Executor registered on agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0
I0310 01:49:29.651634 8372 executor.cpp:176] Received SUBSCRIBED event
I0310 01:49:29.656656 8372 executor.cpp:180] Subscribed executor on build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0310 01:49:29.657655 8372 executor.cpp:176] Received LAUNCH event
I0310 01:49:29.662653 8372 executor.cpp:648] Starting task 54c49790-38d0-4357-8934-e1f431b5f286
I0310 01:49:29.746654 8372 executor.cpp:483] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I0310 01:49:30.289628 8372 executor.cpp:661] Forked command at 7692
I0310 01:49:30.329670 1056 exec.cpp:445] Executor asked to shutdown
I0310 01:49:30.330628 8372 executor.cpp:176] Received SHUTDOWN event
I0310 01:49:30.330628 8372 executor.cpp:758] Shutting down
I0310 01:49:30.330628 8372 executor.cpp:868] Sending SIGTERM to process tree at pid 76-fac8e2084c51-0000 at executor(1)@10.3.1.11:50229
I0310 01:49:30.327628 10884 slave.cpp:924] Agent terminating
W0310 01:49:30.328877 10884 slave.cpp:3874] Ignoring shutdown framework fd8c9e29-7d14-4533-bb86-fac8e2084c51-0000 because it is terminating
I0310 01:49:30.330628 4588 master.cpp:10313] Removing task 54c49790-38d0-4357-8934-e1f431b5f286 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework fd8c9e29-7d14-4533-bb86-fac8e2084c51-0000 on agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0 at slave(398)@10.3.1.11:50208 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0310 01:49:30.331629 4048 containerizer.cpp:2338] Destroying container ff5c8033-55fa-43b1-81e4-58cc54c75678 in RUNNING state
I0310 01:49:30.331629 4048 containerizer.cpp:2952] Transitioning the state of container ff5c8033-55fa-43b1-81e4-58cc54c75678 from RUNNING to DESTROYING
I0310 01:49:30.333627 4048 launcher.cpp:156] Asked to destroy container ff5c8033-55fa-43b1-81e4-58cc54c75678
I0310 01:49:30.333627 4588 master.cpp:1288] Agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0 at slave(398)@10.3.1.11:50208 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0310 01:49:30.333627 11240 hierarchical.cpp:344] Removed framework fd8c9e29-7d14-4533-bb86-fac8e2084c51-0000
I0310 01:49:30.334632 4588 master.cpp:3258] Disconnecting agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0 at slave(398)@10.3.1.11:50208 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0310 01:49:30.335628 4588 master.cpp:3277] Deactivating agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0 at slave(398)@10.3.1.11:50208 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0310 01:49:30.335628 9076 hierarchical.cpp:766] Agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0 deactivated
I0310 01:49:30.376629 4048 containerizer.cpp:2791] Container ff5c8033-55fa-43b1-81e4-58cc54c75678 has exited
I0310 01:49:30.407629 976 master.cpp:1131] Master terminating
I0310 01:49:30.410658 11240 hierarchical.cpp:609] Removed agent fd8c9e29-7d14-4533-bb86-fac8e2084c51-S0
I0310 01:49:31.095378 4164 process.cpp:929] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On March 10, 2018, 12:44 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 10, 2018, 12:44 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On March 12, 2018, 3:29 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1610-1612 (patched)
> > <https://reviews.apache.org/r/66016/diff/2/?file=1973318#file1973318line1620>
> >
> > Hm.. as an aside, looks like `Option::getOrElse` could be improved to avoid copying in cases like this?
> >
> > E.g.
> >
> > ```
> > T&& getOrElse(T&& _t) && { return std::move(isNone() ? _t : t); }
> > T&& getOrElse(T&& _t) const && { return std::move(isNone() ? _t : t); }
> > ```
Sounds good. Will add this in a follow up patch.
> On March 12, 2018, 3:29 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1616 (patched)
> > <https://reviews.apache.org/r/66016/diff/2/?file=1973318#file1973318line1626>
> >
> > This should probably be renamed to s/Resources/ScalarQuantities/ to be consistent with your other naming? Want to do that in a separate patch?
Will do.
> On March 12, 2018, 3:29 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2002-2003 (original), 1972-1973 (patched)
> > <https://reviews.apache.org/r/66016/diff/2/?file=1973318#file1973318line2028>
> >
> > Why isn't this:
> >
> > ```
> > rolesConsumedQuotaScalarQuantites[role] +=
> > newQuotaAllocationScalarQuantities;
> > ```
> >
> > It's not obvious to me, are the two equivalent? The one included in the patch seems to be including resources that aren't part of the quota guarantee? Seems weird?
Both produce the correct result (version 1 of this patch actaully uses this formula). But I think `+= allocatedUnreserved` makes more sense. The way we calculate `rolesConsumedQuotaScalarQuantites` in the first place is by summing `reservations + allocation - allocated reservations` i.e. `quota.contains(rolesConsumedQuotaScalarQuantites)` was never true.
I have plan to persist `rolesConsumedQuotaScalarQuantites` for quota role across iterations. Consider a role updated with a new quota. `+= newQuotaAllocationScalarQuantities` would need to recalculate `rolesConsumedQuotaScalarQuantites` but `+= allocatedUnreserved` would be fine.
In a more general sense, each role has some (default) quota for all resources, consumed resources that only has default quota should still be counted as consumed quota.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review199042
-----------------------------------------------------------
On March 12, 2018, 4:07 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 12, 2018, 4:07 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66016: Refactored resource allocation logic.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66016/#review199042
-----------------------------------------------------------
Good cleanup thanks! Just two issues below that I would like to resolve prior to shipping.
src/master/allocator/mesos/hierarchical.cpp
Lines 1610-1612 (patched)
<https://reviews.apache.org/r/66016/#comment279319>
Hm.. as an aside, looks like `Option::getOrElse` could be improved to avoid copying in cases like this?
E.g.
```
T&& getOrElse(T&& _t) && { return std::move(isNone() ? _t : t); }
T&& getOrElse(T&& _t) const && { return std::move(isNone() ? _t : t); }
```
src/master/allocator/mesos/hierarchical.cpp
Lines 1616 (patched)
<https://reviews.apache.org/r/66016/#comment279320>
This should probably be renamed to s/Resources/ScalarQuantities/ to be consistent with your other naming? Want to do that in a separate patch?
src/master/allocator/mesos/hierarchical.cpp
Lines 2002-2003 (original), 1972-1973 (patched)
<https://reviews.apache.org/r/66016/#comment279321>
Why isn't this:
```
rolesConsumedQuotaScalarQuantites[role] +=
newQuotaAllocationScalarQuantities;
```
It's not obvious to me, are the two equivalent? The one included in the patch seems to be including resources that aren't part of the quota guarantee? Seems weird?
src/master/allocator/mesos/hierarchical.cpp
Lines 2012-2014 (original), 1975-1977 (patched)
<https://reviews.apache.org/r/66016/#comment279322>
Not a new problem, but looks like a comment is warranted here, I couldn't easily figure out why one subtracts new quota allocation and the other subtracts unreserved allocation.
- Benjamin Mahler
On March 10, 2018, 12:44 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66016/
> -----------------------------------------------------------
>
> (Updated March 10, 2018, 12:44 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current allocation logic maintains several states
> such as allocated-reservation, unsatisfied-quota and etc.
> This patch refactors the logic so that we only need to
> maintain a per-quota-role map to track role consumed quota.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 5d30d1d4d4bbb5431745f61b5318b39c5c3a7117
>
>
> Diff: https://reviews.apache.org/r/66016/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>