You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joerg Schad <jo...@mesosphere.io> on 2015/06/26 18:26:58 UTC
Review Request 35927: Added TaskStatus::Reason to Termination Message.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/
-----------------------------------------------------------
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-2035
https://issues.apache.org/jira/browse/MESOS-2035
Repository: mesos
Description
-------
[WIP]
-need to add documentation
-need to add test
-not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
Diffs
-----
docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
Diff: https://reviews.apache.org/r/35927/diff/
Testing
-------
make check (including docker tests)
Thanks,
Joerg Schad
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Jie Yu <yu...@gmail.com>.
> On June 29, 2015, 7:40 p.m., Jie Yu wrote:
> > Thanks Joerg! See my detailed comments. I suggest you take a look at the discussion in https://reviews.apache.org/r/34720/ first.
Hi Joerg, I am wondering if you are still working on this ticket? Some of our internal framework developers noticed that Mesos sends a TASK_FAILED with REASON_MEMORY_LIMIT even if he just specified the wrong executor command (thus causing executor registration timeout). This is very misleading and we want to fix that asap.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89779
-----------------------------------------------------------
On June 29, 2015, 7:22 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 7:22 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b8591116eadcd68b8db2a629fbcf793e6b394f14
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89779
-----------------------------------------------------------
Thanks Joerg! See my detailed comments. I suggest you take a look at the discussion in https://reviews.apache.org/r/34720/ first.
include/mesos/mesos.proto (lines 888 - 889)
<https://reviews.apache.org/r/35927/#comment142583>
Not sure if that's needed. I think REASON_CONTAINER_LAUNCH_ERROR with proper message should be sufficient? cc @tnachen
src/slave/containerizer/docker.cpp (line 1301)
<https://reviews.apache.org/r/35927/#comment142585>
See my above comments. The slave is the one that should set executor->reason to be REASON_CONTAINER_LAUNCH_ERROR. The containerizer just need to properly set the messages.
src/slave/containerizer/mesos/containerizer.cpp (line 1152)
<https://reviews.apache.org/r/35927/#comment142586>
We could follow up with patches to remove 'killed' in both docker and mesos containerizer.
src/slave/slave.cpp (lines 4452 - 4469)
<https://reviews.apache.org/r/35927/#comment142584>
Please take a look at the discussion in this review (2nd comments)
https://reviews.apache.org/r/34720/
There's some subtleness involved for command executors.
The slave also needs to set executor->reason to be REASON_CONTAINER_LAUNCH_ERROR when containerizer->launch returns a failure.
- Jie Yu
On June 29, 2015, 7:22 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 7:22 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b8591116eadcd68b8db2a629fbcf793e6b394f14
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89717
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35927]
All tests passed.
- Mesos ReviewBot
On June 29, 2015, 7:22 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 7:22 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b8591116eadcd68b8db2a629fbcf793e6b394f14
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/
-----------------------------------------------------------
(Updated June 29, 2015, 7:22 a.m.)
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-2035
https://issues.apache.org/jira/browse/MESOS-2035
Repository: mesos
Description
-------
[WIP]
-need to add documentation
-need to add test
-not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
Diffs (updated)
-----
docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
src/slave/slave.cpp b8591116eadcd68b8db2a629fbcf793e6b394f14
src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
Diff: https://reviews.apache.org/r/35927/diff/
Testing
-------
make check (including docker tests)
Thanks,
Joerg Schad
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > include/mesos/containerizer/containerizer.proto, lines 87-89
> > <https://reviews.apache.org/r/35927/diff/1/?file=993029#file993029line87>
> >
> > Why do we need a deprecation cycle for this? AFAIK, it's part of the containerizer API only and I'm not aware of any third-party containers.
>
> Joerg Schad wrote:
> I believe we need more discussion on how that will effect upgradedabilty. Created [Mesos-2954](https://issues.apache.org/jira/browse/MESOS-2954) for further discussion.
Maybe you're right, it's a `required` field. Let's discuss it on JIRA; dropping for now.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > include/mesos/slave/isolator.hpp, line 64
> > <https://reviews.apache.org/r/35927/diff/1/?file=993031#file993031line64>
> >
> > AFAIK, techinally, limitation does not necessarily end in termination, though it is so for `MesosContainerizer`.
> >
> > Also, I'm not sure that reusing `TaskStatus::Reason` enum is a good idea: it includes much more stuff that is not related to limitations.
>
> Joerg Schad wrote:
> If I understand the description of Limitation correctly, it indicates a termination of the executor (see https://github.com/apache/mesos/blob/1a82a3fb2bc717c468218384190a115b770f88c3/include/mesos/slave/isolator.hpp#L42). BTW fixed the typo.
It's true, but take a look at how the isolation is implemented: an isolator watches executor, generates a limitation, which triggers `limited()` callback on containerizer. I still would argue that we should separate two concepts: limitation event and reaction to it (which is termination in case of `MesosContainerizer`, but it's up to `limited()` callback).
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89667
-----------------------------------------------------------
On June 29, 2015, 7:22 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 7:22 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b8591116eadcd68b8db2a629fbcf793e6b394f14
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > include/mesos/slave/isolator.hpp, line 64
> > <https://reviews.apache.org/r/35927/diff/1/?file=993031#file993031line64>
> >
> > AFAIK, techinally, limitation does not necessarily end in termination, though it is so for `MesosContainerizer`.
> >
> > Also, I'm not sure that reusing `TaskStatus::Reason` enum is a good idea: it includes much more stuff that is not related to limitations.
If I understand the description of Limitation correctly, it indicates a termination of the executor (see https://github.com/apache/mesos/blob/1a82a3fb2bc717c468218384190a115b770f88c3/include/mesos/slave/isolator.hpp#L42). BTW fixed the typo.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > include/mesos/containerizer/containerizer.proto, lines 87-89
> > <https://reviews.apache.org/r/35927/diff/1/?file=993029#file993029line87>
> >
> > Why do we need a deprecation cycle for this? AFAIK, it's part of the containerizer API only and I'm not aware of any third-party containers.
I believe we need more discussion on how that will effect upgradedabilty. Created [Mesos-2954](https://issues.apache.org/jira/browse/MESOS-2954) for further discussion.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > src/tests/docker_containerizer_tests.cpp, lines 2777-2778
> > <https://reviews.apache.org/r/35927/diff/1/?file=993037#file993037line2777>
> >
> > Why removing `TODO` wihtout checking for reason?
Will be added with the other tests.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/isolators/posix/disk.cpp, line 253
> > <https://reviews.apache.org/r/35927/diff/1/?file=993034#file993034line253>
> >
> > Is there a test for this case? Or do you plan to add it as part of the WIP?
Test will be added.
> On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/isolators/cgroups/mem.cpp, line 696
> > <https://reviews.apache.org/r/35927/diff/1/?file=993033#file993033line696>
> >
> > Is there a test for this case? Or do you plan to add it as part of the WIP?
Test will be added.
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89667
-----------------------------------------------------------
On June 26, 2015, 4:26 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 4:26 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89667
-----------------------------------------------------------
include/mesos/containerizer/containerizer.proto (lines 87 - 89)
<https://reviews.apache.org/r/35927/#comment142324>
Why do we need a deprecation cycle for this? AFAIK, it's part of the containerizer API only and I'm not aware of any third-party containers.
include/mesos/containerizer/containerizer.proto (line 95)
<https://reviews.apache.org/r/35927/#comment142323>
Add a space.
include/mesos/mesos.proto (lines 886 - 889)
<https://reviews.apache.org/r/35927/#comment142322>
Let's order field tags: 19 -> ... -> 21
include/mesos/slave/isolator.hpp (line 51)
<https://reviews.apache.org/r/35927/#comment142325>
Shouldn't you include `<mesos/mesos.hpp>` header?
include/mesos/slave/isolator.hpp (line 64)
<https://reviews.apache.org/r/35927/#comment142326>
AFAIK, techinally, limitation does not necessarily end in termination, though it is so for `MesosContainerizer`.
Also, I'm not sure that reusing `TaskStatus::Reason` enum is a good idea: it includes much more stuff that is not related to limitations.
src/slave/containerizer/isolators/cgroups/mem.cpp (line 696)
<https://reviews.apache.org/r/35927/#comment142329>
Is there a test for this case? Or do you plan to add it as part of the WIP?
src/slave/containerizer/isolators/posix/disk.cpp (line 253)
<https://reviews.apache.org/r/35927/#comment142327>
Is there a test for this case? Or do you plan to add it as part of the WIP?
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/35927/#comment142330>
Why removing `TODO` wihtout checking for reason?
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/35927/#comment142331>
ditto
- Alexander Rukletsov
On June 26, 2015, 4:26 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 4:26 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 35927: Added TaskStatus::Reason to Termination
Message.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35927/#review89518
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35927]
All tests passed.
- Mesos ReviewBot
On June 26, 2015, 4:26 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35927/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 4:26 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-2035
> https://issues.apache.org/jira/browse/MESOS-2035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> [WIP]
> -need to add documentation
> -need to add test
> -not all potential paths are covered. It is not clear whether isolator sees event before the kill, see first comment in CgroupsMemIsolatorProcess::oom in mem.cpp.
>
>
> Diffs
> -----
>
> docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b
> include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c
> include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19
> include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036
> src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba
> src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5
> src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd
> src/slave/containerizer/mesos/containerizer.cpp 313e9b74d3a0157609226041246575d2c4a503f8
> src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
> src/tests/docker_containerizer_tests.cpp 3a983c6813dab6fa03ccb2c87e1ea71866766d6e
>
> Diff: https://reviews.apache.org/r/35927/diff/
>
>
> Testing
> -------
>
> make check (including docker tests)
>
>
> Thanks,
>
> Joerg Schad
>
>