----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45363/#review137687 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 505) <https://reviews.apache.org/r/45363/#comment202884> I can see two issues here: 1. `updated` is a field in subsystem level (`MemorySubsystem` class) rather than container level (`Info` class), that means once we set it to `true` (e.g., when we update resources for the first container), it will be `true` for all the subsequent container, I do not think that is what we expect. Actually what we need is a flag in container level with which we can know whether it is the first time that we update resources for this container. I think that's the purpose of `CgroupsMemIsolatorProcess::Info::pid` in the original `cgroups/mem` isolator. 2. How to recover this `updated` field when agent is restarted? I think when agent is restarted, this field will be always set back to `false` though it may be `true` before agent is restarted. Actually I see the same issue in the original `cgroups/mem` isolator, in that isolator, I do not see we recover `CgroupsMemIsolatorProcess::Info::pid` in `CgroupsMemIsolatorProcess::recover`. So it is a bug in the `cgroups/mem` isolator? We probably need to follow the way of the `cgroups/mem` isolator (use `CgroupsMemIsolatorProcess::Info::pid` as the flag) and recover `CgroupsMemIsolatorProcess::Info::pid` during recovery (I think we can get pid from `ContainerState`). - Qian Zhang On April 16, 2016, 6:22 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45363/ > ----------------------------------------------------------- > > (Updated April 16, 2016, 6:22 p.m.) > > > Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and > Kevin Klues. > > > Bugs: MESOS-5045 > https://issues.apache.org/jira/browse/MESOS-5045 > > > Repository: mesos > > > Description > ------- > > Add `MemorySubsystem` for cgroups unified isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/45363/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
