You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Clement Michaud <cl...@gmail.com> on 2019/02/10 11:15:42 UTC
Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos
Description
-------
This commit introduces a master hook to decorate task resources in
order to allocate a given amount of custom resource if the framework
does not support it yet.
For instance, if one introduces a new custom resource in a cluster
running frameworks not supporting this resource, there will be a mixed
set of tasks consuming and not consuming this resource leading to
isolation issues. By implementing this hook, a default amount can be
allocated for a custom resources on behalf of the framework so that
every tasks end up consuming this resource and Mesos can take it into
account.
This implicit allocation of resource helps introducing a new custom
resource in the clusters because, before this patch, all frameworks
needed to be patched before introducing the new resource while now a
default value can be applied for the frameworks not supporting the
resource yet meaning the patches can be done later.
https://issues.apache.org/jira/browse/MESOS-9315
Diffs
-----
include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
Diff: https://reviews.apache.org/r/69938/diff/1/
Testing
-------
Thanks,
Clement Michaud
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
> On fév. 12, 2019, 8:21 après-midi, Benjamin Mahler wrote:
> > Ship It!
Awesome! Thank you very much!
- Clement
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212756
-----------------------------------------------------------
On fév. 11, 2019, 10:27 après-midi, Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated fév. 11, 2019, 10:27 après-midi)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212756
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Mahler
On Feb. 11, 2019, 10:27 p.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 10:27 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 12, 2019, 12:33 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> >
> > Reviews applied: `['69938']`
> >
> > Failed command: `Start-MesosCITesting`
> >
> > All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938
> >
> > Relevant logs:
> >
> > - [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938/logs/mesos-tests.log):
> >
> > ```
> > I0211 23:33:18.541034 38112 master.cpp:11334] Removing task 7445aba2-8f7f-4e39-81f3-2dedacf4d400 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000 on agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.543037 44616 master.cpp:1269] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
> > I0211 23:33:18.543037 44616 master.cpp:3272] Disconnecting agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.543037 44616 master.cpp:3291] Deactivating agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.544024 688 hierarchical.cpp:358] Removed framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000
> > I0211 23:33:18.544024 688 hierarchical.cpp:793] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 deactivated
> > I0211 23:33:18.545049 44772 containerizer.cpp:2477] Destroying container a8b74ba0-74f5-42a0-b1f3-5f78d854efae in RUNNING state
> > I0211 23:33:18.545049 44772 containerizer.cpp:3144] Transitioning the state of container a8b74ba0-74f5-42a0-b1f3-5f78d854efae from RUNNING to DESTROYING
> > I0211 23:33:18.546025 44772 launcher.cpp:161] Asked to destroy container a8b74ba0-74f5-42a0-b1f3-5f78d854efae
> > W0211 23:33:18.547099 44612 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=7556 to peer '192.10.1.6:50244': IO failed with error code: The specified network name is no longer available.
> >
> > W0211 23:33:18.54709[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (692 ms)
> > [----------] 1 test from IsolationFlag/MemoryIsolatorTest (708 ms total)
> >
> > [----------] Global test environment tear-down
> > [==========] 1095 tests from 104 test cases ran. (508950 ms total)
> > [ PASSED ] 1094 tests.
> > [ FAILED ] 1 test, listed below:
> > [ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
> >
> > 1 FAILED TEST
> > YOU HAVE 232 DISABLED TESTS
> >
> > 9 44612 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=7880 to peer '192.10.1.6:50245': IO failed with error code: The specified network name is no longer available.
> >
> > I0211 23:33:18.642029 44616 containerizer.cpp:2983] Container a8b74ba0-74f5-42a0-b1f3-5f78d854efae has exited
> > I0211 23:33:18.675004 45404 master.cpp:1109] Master terminating
> > I0211 23:33:18.676004 42772 hierarchical.cpp:644] Removed agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0
> > I0211 23:33:19.212002 44612 process.cpp:927] Stopped the socket accept loop
> > ```
>
> Clement Michaud wrote:
> Weird, can it be a randomness? Can we re-trigger the job?
Created https://issues.apache.org/jira/browse/MESOS-9566.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212725
-----------------------------------------------------------
On Feb. 11, 2019, 11:27 p.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 11:27 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
> On fév. 11, 2019, 11:33 après-midi, Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> >
> > Reviews applied: `['69938']`
> >
> > Failed command: `Start-MesosCITesting`
> >
> > All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938
> >
> > Relevant logs:
> >
> > - [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938/logs/mesos-tests.log):
> >
> > ```
> > I0211 23:33:18.541034 38112 master.cpp:11334] Removing task 7445aba2-8f7f-4e39-81f3-2dedacf4d400 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000 on agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.543037 44616 master.cpp:1269] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
> > I0211 23:33:18.543037 44616 master.cpp:3272] Disconnecting agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.543037 44616 master.cpp:3291] Deactivating agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
> > I0211 23:33:18.544024 688 hierarchical.cpp:358] Removed framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000
> > I0211 23:33:18.544024 688 hierarchical.cpp:793] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 deactivated
> > I0211 23:33:18.545049 44772 containerizer.cpp:2477] Destroying container a8b74ba0-74f5-42a0-b1f3-5f78d854efae in RUNNING state
> > I0211 23:33:18.545049 44772 containerizer.cpp:3144] Transitioning the state of container a8b74ba0-74f5-42a0-b1f3-5f78d854efae from RUNNING to DESTROYING
> > I0211 23:33:18.546025 44772 launcher.cpp:161] Asked to destroy container a8b74ba0-74f5-42a0-b1f3-5f78d854efae
> > W0211 23:33:18.547099 44612 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=7556 to peer '192.10.1.6:50244': IO failed with error code: The specified network name is no longer available.
> >
> > W0211 23:33:18.54709[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (692 ms)
> > [----------] 1 test from IsolationFlag/MemoryIsolatorTest (708 ms total)
> >
> > [----------] Global test environment tear-down
> > [==========] 1095 tests from 104 test cases ran. (508950 ms total)
> > [ PASSED ] 1094 tests.
> > [ FAILED ] 1 test, listed below:
> > [ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
> >
> > 1 FAILED TEST
> > YOU HAVE 232 DISABLED TESTS
> >
> > 9 44612 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=7880 to peer '192.10.1.6:50245': IO failed with error code: The specified network name is no longer available.
> >
> > I0211 23:33:18.642029 44616 containerizer.cpp:2983] Container a8b74ba0-74f5-42a0-b1f3-5f78d854efae has exited
> > I0211 23:33:18.675004 45404 master.cpp:1109] Master terminating
> > I0211 23:33:18.676004 42772 hierarchical.cpp:644] Removed agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0
> > I0211 23:33:19.212002 44612 process.cpp:927] Stopped the socket accept loop
> > ```
Weird, can it be a randomness? Can we re-trigger the job?
- Clement
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212725
-----------------------------------------------------------
On fév. 11, 2019, 10:27 après-midi, Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated fév. 11, 2019, 10:27 après-midi)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212725
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['69938']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2875/mesos-review-69938/logs/mesos-tests.log):
```
I0211 23:33:18.541034 38112 master.cpp:11334] Removing task 7445aba2-8f7f-4e39-81f3-2dedacf4d400 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000 on agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0211 23:33:18.543037 44616 master.cpp:1269] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0211 23:33:18.543037 44616 master.cpp:3272] Disconnecting agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0211 23:33:18.543037 44616 master.cpp:3291] Deactivating agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 at slave(477)@192.10.1.6:64710 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0211 23:33:18.544024 688 hierarchical.cpp:358] Removed framework 5dbd2f44-60f7-4adc-abef-b7d101aa059c-0000
I0211 23:33:18.544024 688 hierarchical.cpp:793] Agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0 deactivated
I0211 23:33:18.545049 44772 containerizer.cpp:2477] Destroying container a8b74ba0-74f5-42a0-b1f3-5f78d854efae in RUNNING state
I0211 23:33:18.545049 44772 containerizer.cpp:3144] Transitioning the state of container a8b74ba0-74f5-42a0-b1f3-5f78d854efae from RUNNING to DESTROYING
I0211 23:33:18.546025 44772 launcher.cpp:161] Asked to destroy container a8b74ba0-74f5-42a0-b1f3-5f78d854efae
W0211 23:33:18.547099 44612 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=7556 to peer '192.10.1.6:50244': IO failed with error code: The specified network name is no longer available.
W0211 23:33:18.54709[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (692 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (708 ms total)
[----------] Global test environment tear-down
[==========] 1095 tests from 104 test cases ran. (508950 ms total)
[ PASSED ] 1094 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
1 FAILED TEST
YOU HAVE 232 DISABLED TESTS
9 44612 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=7880 to peer '192.10.1.6:50245': IO failed with error code: The specified network name is no longer available.
I0211 23:33:18.642029 44616 containerizer.cpp:2983] Container a8b74ba0-74f5-42a0-b1f3-5f78d854efae has exited
I0211 23:33:18.675004 45404 master.cpp:1109] Master terminating
I0211 23:33:18.676004 42772 hierarchical.cpp:644] Removed agent 5dbd2f44-60f7-4adc-abef-b7d101aa059c-S0
I0211 23:33:19.212002 44612 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On Feb. 11, 2019, 10:27 p.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 10:27 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/
-----------------------------------------------------------
(Updated fév. 11, 2019, 10:27 après-midi)
Review request for mesos and Benjamin Mahler.
Changes
-------
Fix issue with implicit conversion.
Bugs: MESOS-9315
https://issues.apache.org/jira/browse/MESOS-9315
Repository: mesos
Description
-------
This commit introduces a master hook to decorate task resources in
order to allocate a given amount of custom resource if the framework
does not support it yet.
For instance, if one introduces a new custom resource in a cluster
running frameworks not supporting this resource, there will be a mixed
set of tasks consuming and not consuming this resource leading to
isolation issues. By implementing this hook, a default amount can be
allocated for a custom resources on behalf of the framework so that
every tasks end up consuming this resource and Mesos can take it into
account.
This implicit allocation of resource helps introducing a new custom
resource in the clusters because, before this patch, all frameworks
needed to be patched before introducing the new resource while now a
default value can be applied for the frameworks not supporting the
resource yet meaning the patches can be done later.
https://issues.apache.org/jira/browse/MESOS-9315
Diffs (updated)
-----
include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
Diff: https://reviews.apache.org/r/69938/diff/3/
Changes: https://reviews.apache.org/r/69938/diff/2-3/
Testing
-------
I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
Thanks,
Clement Michaud
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212723
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['69938']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2874/mesos-review-69938
Relevant logs:
- [mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2874/mesos-review-69938/logs/mesos-tests-cmake.log):
```
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500): warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501): warning C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479): warning C4101: 'addrstr': unreferenced local variable [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170): warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496): warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256): warning C4090: 'function': different 'const' qualifiers [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166): warning C4716: 'pthread_cond_broadcast': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205): warning C4716: 'pthread_cond_wait': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
"D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
(ClCompile target) ->
d:\dcos\mesos\mesos\src\tests\hook_tests.cpp(298): error C2039: 'get': is not a member of 'google::protobuf::RepeatedPtrField<mesos::Resource>' [D:\DCOS\mesos\src\tests\mesos-tests.vcxproj]
172 Warning(s)
1 Error(s)
Time Elapsed 00:29:26.14
```
- Mesos Reviewbot Windows
On Feb. 11, 2019, 8:50 p.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 8:50 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/2/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/
-----------------------------------------------------------
(Updated fév. 11, 2019, 8:50 après-midi)
Review request for mesos and Benjamin Mahler.
Changes
-------
Fixed all comments.
Bugs: MESOS-9315
https://issues.apache.org/jira/browse/MESOS-9315
Repository: mesos
Description
-------
This commit introduces a master hook to decorate task resources in
order to allocate a given amount of custom resource if the framework
does not support it yet.
For instance, if one introduces a new custom resource in a cluster
running frameworks not supporting this resource, there will be a mixed
set of tasks consuming and not consuming this resource leading to
isolation issues. By implementing this hook, a default amount can be
allocated for a custom resources on behalf of the framework so that
every tasks end up consuming this resource and Mesos can take it into
account.
This implicit allocation of resource helps introducing a new custom
resource in the clusters because, before this patch, all frameworks
needed to be patched before introducing the new resource while now a
default value can be applied for the frameworks not supporting the
resource yet meaning the patches can be done later.
https://issues.apache.org/jira/browse/MESOS-9315
Diffs (updated)
-----
include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
Diff: https://reviews.apache.org/r/69938/diff/2/
Changes: https://reviews.apache.org/r/69938/diff/1-2/
Testing
-------
I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
Thanks,
Clement Michaud
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Feb. 11, 2019, 6:58 p.m., Benjamin Mahler wrote:
> > Nice patch, thanks Clément. Just a few minor comments below and we should be good to go.
> >
> > Just as an aside, the current hook interfaces are rather inefficient (copies the entire task info). If you're interested in improving the efficiency of the hook interface let me know and we can tackle it separately.
>
> Clement Michaud wrote:
> Do you suggest we update the resources one by one instead of using a TaskInfo copy?
To make it zero copy, we'll have to change the hook interface, a suggestion was left at the bottom of https://issues.apache.org/jira/browse/MESOS-9336
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212704
-----------------------------------------------------------
On Feb. 11, 2019, 10:27 p.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 10:27 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/3/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > include/mesos/hook.hpp
> > Lines 56-62 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124662#file2124662line56>
> >
> > It's not clear from here what the semantics are, can you add:
> >
> > ```
> > These new resources overwrite the previous ones in TaskInfo.
> > ```
sure.
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > src/examples/test_hook_module.cpp
> > Lines 114-148 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124663#file2124663line114>
> >
> > how about:
> >
> > ```
> > // The test logic here is based on the use case of injecting
> > // default network bandwidth resource.
> > Result<Resources> masterLaunchTaskResourceDecorator(
> > const TaskInfo& taskInfo,
> > const Resources& slaveResources) override
> > {
> > LOG(INFO) << "Executing 'masterLaunchTaskResourceDecorator' hook";
> >
> > // If slave does not declare network bandwidth resource,
> > // don't set a default value for it.
> > if (slaveResources.names().count("network_bandwidth") == 0) {
> > return taskInfo.resources();
> > }
> >
> > Resources taskResources = taskInfo.resources();
> >
> > // Add a default value for network bandwidth if absent.
> > if (taskResources.names().count("network_bandwidth") == 0) {
> > taskResources += CHECK_NOTERROR(Resources::parse("network_bandwidth:10"));
> > }
> >
> > return taskResources;
> > }
> > ```
Much shorter, I like that.
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > src/hook/manager.cpp
> > Lines 149 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124665#file2124665line149>
> >
> > Since we want to move `result`, it should not be const? I'm surprised it compiles, I suspect the const renders the move below into a copy operation?
You are right!
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 4459-4461 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124666#file2124666line4459>
> >
> > Let's avoid the copy:
> >
> > ```
> > *task.mutable_resources() =
> > HookManager::masterLaunchTaskResourceDecorator(
> > task,
> > slave->totalResources));
> > ```
done
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 4482-4484 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124666#file2124666line4482>
> >
> > Let's avoid the copy:
> >
> > ```
> > *task.mutable_resources() =
> > HookManager::masterLaunchTaskResourceDecorator(
> > task,
> > slave->totalResources));
> > ```
done
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > src/tests/hook_tests.cpp
> > Lines 246-247 (patched)
> > <https://reviews.apache.org/r/69938/diff/1/?file=2124667#file2124667line246>
> >
> > We can simplify this test by not using the mock executor and test containerizer, and removing all the expectations around it below.
> >
> > Was it added because of copy / paste?
This was indeed a copy paste.
- Clement
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212704
-----------------------------------------------------------
On fév. 10, 2019, 11:16 matin, Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated fév. 10, 2019, 11:16 matin)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/1/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
> On fév. 11, 2019, 6:58 après-midi, Benjamin Mahler wrote:
> > Nice patch, thanks Clément. Just a few minor comments below and we should be good to go.
> >
> > Just as an aside, the current hook interfaces are rather inefficient (copies the entire task info). If you're interested in improving the efficiency of the hook interface let me know and we can tackle it separately.
Do you suggest we update the resources one by one instead of using a TaskInfo copy?
- Clement
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212704
-----------------------------------------------------------
On fév. 11, 2019, 8:50 après-midi, Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated fév. 11, 2019, 8:50 après-midi)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/2/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212704
-----------------------------------------------------------
Nice patch, thanks Clément. Just a few minor comments below and we should be good to go.
Just as an aside, the current hook interfaces are rather inefficient (copies the entire task info). If you're interested in improving the efficiency of the hook interface let me know and we can tackle it separately.
include/mesos/hook.hpp
Lines 56-62 (patched)
<https://reviews.apache.org/r/69938/#comment298582>
It's not clear from here what the semantics are, can you add:
```
These new resources overwrite the previous ones in TaskInfo.
```
src/examples/test_hook_module.cpp
Lines 114-148 (patched)
<https://reviews.apache.org/r/69938/#comment298588>
how about:
```
// The test logic here is based on the use case of injecting
// default network bandwidth resource.
Result<Resources> masterLaunchTaskResourceDecorator(
const TaskInfo& taskInfo,
const Resources& slaveResources) override
{
LOG(INFO) << "Executing 'masterLaunchTaskResourceDecorator' hook";
// If slave does not declare network bandwidth resource,
// don't set a default value for it.
if (slaveResources.names().count("network_bandwidth") == 0) {
return taskInfo.resources();
}
Resources taskResources = taskInfo.resources();
// Add a default value for network bandwidth if absent.
if (taskResources.names().count("network_bandwidth") == 0) {
taskResources += CHECK_NOTERROR(Resources::parse("network_bandwidth:10"));
}
return taskResources;
}
```
src/hook/manager.cpp
Lines 149 (patched)
<https://reviews.apache.org/r/69938/#comment298589>
Since we want to move `result`, it should not be const? I'm surprised it compiles, I suspect the const renders the move below into a copy operation?
src/master/master.cpp
Lines 4459-4461 (patched)
<https://reviews.apache.org/r/69938/#comment298590>
Let's avoid the copy:
```
*task.mutable_resources() =
HookManager::masterLaunchTaskResourceDecorator(
task,
slave->totalResources));
```
src/master/master.cpp
Lines 4482-4484 (patched)
<https://reviews.apache.org/r/69938/#comment298591>
Let's avoid the copy:
```
*task.mutable_resources() =
HookManager::masterLaunchTaskResourceDecorator(
task,
slave->totalResources));
```
src/tests/hook_tests.cpp
Lines 246-247 (patched)
<https://reviews.apache.org/r/69938/#comment298592>
We can simplify this test by not using the mock executor and test containerizer, and removing all the expectations around it below.
Was it added because of copy / paste?
src/tests/hook_tests.cpp
Lines 306 (patched)
<https://reviews.apache.org/r/69938/#comment298594>
const Resources& (we put the reference next to the type)
or
const Resources
or
Resources
would all be ok here
src/tests/hook_tests.cpp
Lines 307-309 (patched)
<https://reviews.apache.org/r/69938/#comment298593>
Doesn't look like this is needed, the option being none will be caught below
- Benjamin Mahler
On Feb. 10, 2019, 11:16 a.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2019, 11:16 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/1/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/#review212695
-----------------------------------------------------------
PASS: Mesos patch 69938 was successfully built and tested.
Reviews applied: `['69938']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2870/mesos-review-69938
- Mesos Reviewbot Windows
On Feb. 10, 2019, 11:16 a.m., Clement Michaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69938/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2019, 11:16 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9315
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a master hook to decorate task resources in
> order to allocate a given amount of custom resource if the framework
> does not support it yet.
>
> For instance, if one introduces a new custom resource in a cluster
> running frameworks not supporting this resource, there will be a mixed
> set of tasks consuming and not consuming this resource leading to
> isolation issues. By implementing this hook, a default amount can be
> allocated for a custom resources on behalf of the framework so that
> every tasks end up consuming this resource and Mesos can take it into
> account.
>
> This implicit allocation of resource helps introducing a new custom
> resource in the clusters because, before this patch, all frameworks
> needed to be patched before introducing the new resource while now a
> default value can be applied for the frameworks not supporting the
> resource yet meaning the patches can be done later.
>
> https://issues.apache.org/jira/browse/MESOS-9315
>
>
> Diffs
> -----
>
> include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
> src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
> src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
> src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
>
>
> Diff: https://reviews.apache.org/r/69938/diff/1/
>
>
> Testing
> -------
>
> I added a test showing that if a task was missing network_bandwidth resource in the TaskInfo, the hook injects a default value on behalf of the framework.
>
>
> Thanks,
>
> Clement Michaud
>
>
Re: Review Request 69938: Add resource decorator hook to implicitly
allocate mandatory resources.
Posted by Clement Michaud <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69938/
-----------------------------------------------------------
(Updated fév. 10, 2019, 11:16 matin)
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-9315
https://issues.apache.org/jira/browse/MESOS-9315
Repository: mesos
Description
-------
This commit introduces a master hook to decorate task resources in
order to allocate a given amount of custom resource if the framework
does not support it yet.
For instance, if one introduces a new custom resource in a cluster
running frameworks not supporting this resource, there will be a mixed
set of tasks consuming and not consuming this resource leading to
isolation issues. By implementing this hook, a default amount can be
allocated for a custom resources on behalf of the framework so that
every tasks end up consuming this resource and Mesos can take it into
account.
This implicit allocation of resource helps introducing a new custom
resource in the clusters because, before this patch, all frameworks
needed to be patched before introducing the new resource while now a
default value can be applied for the frameworks not supporting the
resource yet meaning the patches can be done later.
https://issues.apache.org/jira/browse/MESOS-9315
Diffs
-----
include/mesos/hook.hpp 019887095e7845d5a65d133b0f58091d262ec55b
src/examples/test_hook_module.cpp c4f449512a4cc150de8a99f44a525b96a2fc1ae2
src/hook/manager.hpp b3d4f5198588068d3b28a57cffb3754b55e33b51
src/hook/manager.cpp 3e71a26f8c0fcfefecc93d70f8a9d6c2d7fdcc6c
src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
src/tests/hook_tests.cpp d8aa35e0027d589044bb131b460311721bd36609
Diff: https://reviews.apache.org/r/69938/diff/1/
Testing
-------
Thanks,
Clement Michaud