> 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 > >
