You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2018/09/05 22:15:11 UTC
Review Request 68637: Avoided common-case extra copying of Resources
in Master::offer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/
-----------------------------------------------------------
Review request for mesos, Gastón Kleiman and Meng Zhu.
Repository: mesos
Description
-------
It's more typical for there not to be ephemeral ports resources,
therefore we avoid copying in the common case.
Diffs
-----
src/master/master.cpp e307df001fba3afbd128c3c0d9db955d6777186a
Diff: https://reviews.apache.org/r/68637/diff/1/
Testing
-------
make check
Thanks,
Benjamin Mahler
Re: Review Request 68637: Avoided extra copying of Resources in
Master::offer.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/#review208429
-----------------------------------------------------------
Ship it!
Ship It!
- Meng Zhu
On Sept. 6, 2018, 3:05 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68637/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 3:05 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Meng Zhu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to remove ephemeral ports resources, we now perform an
> efficient swap to last + remove last operation.
>
>
> Diffs
> -----
>
> src/master/master.cpp 3277b573b78d68a36d98bc8bee47f7a94a5aff18
>
>
> Diff: https://reviews.apache.org/r/68637/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 68637: Avoided extra copying of Resources in
Master::offer.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/#review208430
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['68637']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2305/mesos-review-68637
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2305/mesos-review-68637/logs/mesos-tests.log):
```
I0906 23:05:41.102778 10580 executor.cpp:796] Shutting down
I0906 23:05:41.102778 10580 executor.cpp:909] Sending SIGTERM to process tree at pid 78200 at executor(1)@192.10.1.5:62253
I0906 23:05:41.102778 9260 slave.cpp:909] Agent terminating
W0906 23:05:41.102778 9260 slave.cpp:3917] Ignoring shutdown framework 59aa0497-24a8-4770-be6a-c309f80fbc07-0000 because it is terminating
I0906 23:05:41.102778 13420 master.cpp:11030] Removing task 61fadfb7-4c16-460d-bc4a-75dc92669ddc with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 59aa0497-24a8-4770-be6a-c309f80fbc07-0000 on agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0 at slave(462)@192.10.1.5:60456 (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0906 23:05:41.105769 13420 master.cpp:1251] Agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0 at slave(462)@192.10.1.5:60456 (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I0906 23:05:41.105769 13420 master.cpp:3267] Disconnecting agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0 at slave(462)@192.10.1.5:60456 (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0906 23:05:41.105769 13420 master.cpp:3286] Deactivating agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0 at slave(462)@192.10.1.5:60456 (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0906 23:05:41.106786 13000 hierarchical.cpp:359] Removed framework 59aa0497-24a8-4770-be6a-c309f80fbc07-0000
I0906 23:05:41.106786 13000 hierarchical.cpp:795] Agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0 deactivated
I0906 23:05:41.107771 13420 containerizer.cpp:2455] Destroying container 9493fb9d-697f-416a-8979-2a164e62adfa in RUNNING state
I0906 23:05:41.107771 13420 containerizer.cpp:3118] Transitioning the state of container 9493fb9d-697f-416a-8979-2a164e62adfa from RUNNING to DESTROYING
I0906 23:05:41.107771 13420 launcher.cpp:166] Asked to destroy container 9493fb9d-697f-416a-8979-2a164e62adfa
I0906 23:05:41.120775 9308 containerizer.cpp:2957] Container 9493fb9d-697f-416a-8979-2a164e62adfa has exited
I0906 23:05:41.148789 13604 master.cpp:1093] Master terminating
I0906 23:05:[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (584 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (603 ms total)
[----------] Global test environment tear-down
[==========] 1051 tests from 103 test cases ran. (491325 ms total)
[ PASSED ] 1050 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
1 FAILED TEST
YOU HAVE 229 DISABLED TESTS
41.150792 9260 hierarchical.cpp:637] Removed agent 59aa0497-24a8-4770-be6a-c309f80fbc07-S0
I0906 23:05:41.427780 13532 process.cpp:926] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On Sept. 6, 2018, 3:05 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68637/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 3:05 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Meng Zhu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to remove ephemeral ports resources, we now perform an
> efficient swap to last + remove last operation.
>
>
> Diffs
> -----
>
> src/master/master.cpp 3277b573b78d68a36d98bc8bee47f7a94a5aff18
>
>
> Diff: https://reviews.apache.org/r/68637/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 68637: Avoided extra copying of Resources in
Master::offer.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/
-----------------------------------------------------------
(Updated Sept. 6, 2018, 10:05 p.m.)
Review request for mesos, Gastón Kleiman and Meng Zhu.
Changes
-------
Updated to use swap to last + remove last.
Summary (updated)
-----------------
Avoided extra copying of Resources in Master::offer.
Repository: mesos
Description (updated)
-------
In order to remove ephemeral ports resources, we now perform an
efficient swap to last + remove last operation.
Diffs (updated)
-----
src/master/master.cpp 3277b573b78d68a36d98bc8bee47f7a94a5aff18
Diff: https://reviews.apache.org/r/68637/diff/2/
Changes: https://reviews.apache.org/r/68637/diff/1-2/
Testing
-------
make check
Thanks,
Benjamin Mahler
Re: Review Request 68637: Avoided common-case extra copying of
Resources in Master::offer.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Sept. 6, 2018, 4:42 a.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Line 9379 (original), 9378 (patched)
> > <https://reviews.apache.org/r/68637/diff/1/?file=2082766#file2082766line9379>
> >
> > `DeleteSubrange` will affect all the elements after `i`, this may cause quite some overhead.
> >
> > We can swap `ephemeral_ports` resources to the tail and `DeleteSubrange` them in one go.
> >
> > I do not know how often ephemeral ports are used, but the above approach seems to be worthwhile and would not hurt readability much.
Yeah I originally was planning to do a Swap+RemoveLast approach, but didn't think I could write the code cleanly. It does seem like trivial tweak to the current patch, I'll send out an update.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/#review208388
-----------------------------------------------------------
On Sept. 5, 2018, 10:15 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68637/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 10:15 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Meng Zhu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It's more typical for there not to be ephemeral ports resources,
> therefore we avoid copying in the common case.
>
>
> Diffs
> -----
>
> src/master/master.cpp e307df001fba3afbd128c3c0d9db955d6777186a
>
>
> Diff: https://reviews.apache.org/r/68637/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 68637: Avoided common-case extra copying of
Resources in Master::offer.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/#review208388
-----------------------------------------------------------
src/master/master.cpp
Line 9379 (original), 9378 (patched)
<https://reviews.apache.org/r/68637/#comment292286>
`DeleteSubrange` will affect all the elements after `i`, this may cause quite some overhead.
We can swap `ephemeral_ports` resources to the tail and `DeleteSubrange` them in one go.
I do not know how often ephemeral ports are used, but the above approach seems to be worthwhile and would not hurt readability much.
- Meng Zhu
On Sept. 5, 2018, 3:15 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68637/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 3:15 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Meng Zhu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It's more typical for there not to be ephemeral ports resources,
> therefore we avoid copying in the common case.
>
>
> Diffs
> -----
>
> src/master/master.cpp e307df001fba3afbd128c3c0d9db955d6777186a
>
>
> Diff: https://reviews.apache.org/r/68637/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 68637: Avoided common-case extra copying of
Resources in Master::offer.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68637/#review208390
-----------------------------------------------------------
PASS: Mesos patch 68637 was successfully built and tested.
Reviews applied: `['68634', '68635', '68636', '68637']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2298/mesos-review-68637
- Mesos Reviewbot Windows
On Sept. 6, 2018, 6:15 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68637/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 6:15 a.m.)
>
>
> Review request for mesos, Gastón Kleiman and Meng Zhu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It's more typical for there not to be ephemeral ports resources,
> therefore we avoid copying in the common case.
>
>
> Diffs
> -----
>
> src/master/master.cpp e307df001fba3afbd128c3c0d9db955d6777186a
>
>
> Diff: https://reviews.apache.org/r/68637/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>