> On July 10, 2015, 8:50 p.m., Timothy Chen wrote: > > src/slave/containerizer/docker.cpp, line 1266 > > <https://reviews.apache.org/r/36326/diff/4/?file=1004334#file1004334line1266> > > > > I think you reordered things here that used to be in _usage. > > > > We used to first check the containers map, then check is it destroying, > > that look at the pid and finally passing that on. > > > > Here the order is now pid check, and some reason skip the map check to > > get it but then checking it later on. > > > > I think we should go back to the exact same order as before unless you > > have reasons to do this change.
The thought was as follows: - We called inspect to get the pid. So first we check for a pid. Then we set the pid for the container (if the container exists). Then we call "collectUsage" to collect stats. - The check for DESTROYING is already being centralized in the collectUsage lambda. > On July 10, 2015, 8:50 p.m., Timothy Chen wrote: > > src/slave/containerizer/docker.cpp, line 1272 > > <https://reviews.apache.org/r/36326/diff/4/?file=1004334#file1004334line1272> > > > > If you like to a leave a comment why we called this function we should > > probably comment at the place we call it not in the function (before > > docker->inspect). > > > > also s/didnt/didn't/g Wanted to emphasize the logic of why we need to update the pid there (and not say in collectUSage). > On July 10, 2015, 8:50 p.m., Timothy Chen wrote: > > src/slave/containerizer/docker.cpp, line 1342 > > <https://reviews.apache.org/r/36326/diff/4/?file=1004334#file1004334line1342> > > > > This seems too verbose being LOG(INFO). > > And what value do you think we have here logging this? I can change it to LOG(DEBUG). Wanted to log there to help us debug. - Jojy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36326/#review91342 ----------------------------------------------------------- On July 9, 2015, 8:38 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36326/ > ----------------------------------------------------------- > > (Updated July 9, 2015, 8:38 p.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-2951 > https://issues.apache.org/jira/browse/MESOS-2951 > > > Repository: mesos > > > Description > ------- > > WIP > > Adding cgroups cpustat and memory statistics as preferred way to get usage > statistics for containerizers. > > Jira: MESOS-2951 > > > Diffs > ----- > > src/slave/containerizer/docker.hpp 646a277be21deaded47324bea5474bd68699f25f > src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba > > Diff: https://reviews.apache.org/r/36326/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jojy Varghese > >
