> On March 17, 2015, 6:42 p.m., Ian Downes wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 967-969 > > <https://reviews.apache.org/r/32123/diff/2/?file=896257#file896257line967> > > > > It wasn't a race before was it? I thought the previous behavior was the > > cleanup would get skipped if the container was preparing and the relevant > > Container struct would get dropped, leaking resources.
True, I think it's more of the timing problem not a real race. I'll fix this. > On March 17, 2015, 6:42 p.m., Ian Downes wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1170 > > <https://reviews.apache.org/r/32123/diff/2/?file=896257#file896257line1170> > > > > so killed means both instructed to be killed and killed because of a > > limitation? Yes, according to the existing code in the containerizer. Destroy used to set killed to true when limitations were found. Interestingly is that killed is not even set true before when destroy is called by the slave. I've fixed it as well in this patch. > On March 17, 2015, 6:42 p.m., Ian Downes wrote: > > src/slave/containerizer/mesos/containerizer.hpp, line 212 > > <https://reviews.apache.org/r/32123/diff/2/?file=896256#file896256line212> > > > > IIUC killed=true here if it has been killed externally by the slave > > calling destroy() rather than being killed internally as a result of a > > limitation. But later we set killed=true in the Termination to indicate it > > was killed by a limitation...? > > > > Can you document what killed means and perhaps rename it, if my > > understanding above is correct? That is what killed means in the mesos containerizer. I'm not sure I have a better name for it though, and killed is what we eventually set in Termination so I think it makes sense to keep the flag as-is. I'll just add some comments what killed means. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32123/#review76757 ----------------------------------------------------------- On March 16, 2015, 7:30 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32123/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 7:30 p.m.) > > > Review request for mesos, Ian Downes and Jie Yu. > > > Repository: mesos > > > Description > ------- > > Fix destroying containerizer during isolator prepare. > This ensures the prepare phase is completed before destroying the container. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.hpp > ae61a0fcd19f2ba808624312401f020121baf5d4 > src/slave/containerizer/mesos/containerizer.cpp > fbd1c0a0e5f4f227adb022f0baaa6d2c7e3ad748 > src/tests/containerizer_tests.cpp 565903b754bbf5891891b183a283aee038eb1b8f > > Diff: https://reviews.apache.org/r/32123/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
