You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/09/21 17:47:33 UTC
Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
Bugs: MESOS-7643
https://issues.apache.org/jira/browse/MESOS-7643
Repository: mesos
Description
-------
Historically, isolators were invoked in the order listed by
the operator in the `--isolation` flag. This changed to an
internal ordering in 6fb9c024, back to the operator ordering
in af474a6fa, and then to an undefined ordering in 9642d3c67b.
This commit switches back to an internal ordering for all the built-in
isolators. Custom isolators loaded in modules are run in operator order
after all the built-in ones. The rationale for an internal ordering is
expressed in MESOS-5581; basically we should not burden the operator
with having to figure out how to make the order correct. In the case of
custom isolators there's no way for us to know the correct ordering so
we make an arbitrary choice.
Diffs
-----
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
Diff: https://reviews.apache.org/r/62472/diff/1/
Testing
-------
make check (Fedora 26)
Thanks,
James Peach
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review185941
-----------------------------------------------------------
Patch looks great!
Reviews applied: [62472]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Sept. 21, 2017, 5:47 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2017, 5:47 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
>
>
> Diff: https://reviews.apache.org/r/62472/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review201447
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp
Line 175 (original), 177 (patched)
<https://reviews.apache.org/r/62472/#comment282685>
Remove an extra space between ] and (.
src/slave/containerizer/mesos/containerizer.cpp
Lines 193 (patched)
<https://reviews.apache.org/r/62472/#comment282686>
ditto.
src/slave/containerizer/mesos/containerizer.cpp
Lines 203 (patched)
<https://reviews.apache.org/r/62472/#comment282687>
ditto.
src/slave/containerizer/mesos/containerizer.cpp
Lines 210 (patched)
<https://reviews.apache.org/r/62472/#comment282688>
Should we have this `,`?
src/slave/containerizer/mesos/containerizer.cpp
Lines 342-343 (original), 349-350 (patched)
<https://reviews.apache.org/r/62472/#comment282745>
the order is different in the code below, should we switch the comment?
src/slave/containerizer/mesos/containerizer.cpp
Line 345 (original), 352 (patched)
<https://reviews.apache.org/r/62472/#comment282746>
add gpu isolator?
src/slave/containerizer/mesos/containerizer.cpp
Lines 375 (patched)
<https://reviews.apache.org/r/62472/#comment282743>
typo?
src/slave/containerizer/mesos/containerizer.cpp
Lines 432 (patched)
<https://reviews.apache.org/r/62472/#comment282747>
This is my fault from previous TODO. could you do:
s/will/will not/g?
src/slave/containerizer/mesos/containerizer.cpp
Lines 426-430 (original), 446-450 (patched)
<https://reviews.apache.org/r/62472/#comment282744>
add comment for netowrk isolators?
- Gilbert Song
On Sept. 21, 2017, 10:47 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2017, 10:47 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
>
>
> Diff: https://reviews.apache.org/r/62472/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review201581
-----------------------------------------------------------
PASS: Mesos patch 62472 was successfully built and tested.
Reviews applied: `['62472']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62472
- Mesos Reviewbot Windows
On April 19, 2018, 2:22 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated April 19, 2018, 2:22 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 6568126252a0a8ff5f6095bedae7828a9a1ba0fd
>
>
> Diff: https://reviews.apache.org/r/62472/diff/2/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review201608
-----------------------------------------------------------
Patch looks great!
Reviews applied: [62472]
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 April 19, 2018, 9:22 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated April 19, 2018, 9:22 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 6568126252a0a8ff5f6095bedae7828a9a1ba0fd
>
>
> Diff: https://reviews.apache.org/r/62472/diff/2/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review201641
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On April 19, 2018, 2:22 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated April 19, 2018, 2:22 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 6568126252a0a8ff5f6095bedae7828a9a1ba0fd
>
>
> Diff: https://reviews.apache.org/r/62472/diff/2/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/
-----------------------------------------------------------
(Updated April 19, 2018, 9:22 p.m.)
Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
Bugs: MESOS-7643
https://issues.apache.org/jira/browse/MESOS-7643
Repository: mesos
Description
-------
Historically, isolators were invoked in the order listed by
the operator in the `--isolation` flag. This changed to an
internal ordering in 6fb9c024, back to the operator ordering
in af474a6fa, and then to an undefined ordering in 9642d3c67b.
This commit switches back to an internal ordering for all the built-in
isolators. Custom isolators loaded in modules are run in operator order
after all the built-in ones. The rationale for an internal ordering is
expressed in MESOS-5581; basically we should not burden the operator
with having to figure out how to make the order correct. In the case of
custom isolators there's no way for us to know the correct ordering so
we make an arbitrary choice.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.cpp 6568126252a0a8ff5f6095bedae7828a9a1ba0fd
Diff: https://reviews.apache.org/r/62472/diff/2/
Changes: https://reviews.apache.org/r/62472/diff/1-2/
Testing
-------
make check (Fedora 26)
Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
Thanks,
James Peach
Re: Review Request 62472: Fixed the ordering of Mesos containerizer
isolators.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62472/#review185923
-----------------------------------------------------------
PASS: Mesos patch 62472 was successfully built and tested.
Reviews applied: `['62472']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62472
- Mesos Reviewbot Windows
On Sept. 21, 2017, 7:47 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2017, 7:47 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
>
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
>
>
> Diff: https://reviews.apache.org/r/62472/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 26)
>
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it on users@ and dev@ and update the documentation.
>
>
> Thanks,
>
> James Peach
>
>