> On March 25, 2015, 11 p.m., Chi Zhang wrote: > > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1414 > > <https://reviews.apache.org/r/32426/diff/2/?file=905705#file905705line1414> > > > > Here in recover, the first thing we look at is the veths on the system > > to determine _all_ candidates for recover or orphan cleanup. This is why > > veth used to be cleaned up as the last step in '_cleanup'. > > > > I think you need to keep that in '_cleanup'. Maybe you can re-adjust > > the creation order in this patch to add veth, touch pid file, mount pid > > file and touch symlink in isolate, and do the reverse in '_cleanup'?
See my comments above. I don't want to re-adjust the creation order because the veth will be gone if the child process exits early. I would rather avoid that bahavior. As I already pointed out, stale bind mount is inevitable. What we need is a more robust cleanup logic during recovery for those stale bind mounts (MESOS-2547). Dropping this issue. > On March 25, 2015, 11 p.m., Chi Zhang wrote: > > src/slave/containerizer/isolators/network/port_mapping.cpp, line 1431 > > <https://reviews.apache.org/r/32426/diff/2/?file=905705#file905705line1431> > > > > I have to agree with Vinod that the linker and linkee made it quite > > hard for me to understand too. > > > > Can we use more explicit names like pidToContainerIdMap (just an > > example)? We use linker-->linkee concept in libprocess as well https://github.com/apache/mesos/blob/master/3rdparty/libprocess/src/process.cpp#L303 I could add a comment. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32426/#review77818 ----------------------------------------------------------- On March 25, 2015, 6:43 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32426/ > ----------------------------------------------------------- > > (Updated March 25, 2015, 6:43 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Vinod Kone. > > > Bugs: MESOS-2528 > https://issues.apache.org/jira/browse/MESOS-2528 > > > Repository: mesos > > > Description > ------- > > Symlink the namespace handle with ContainerID for the port mapping isolator. > > See ticket for details. This patch will allow a smooth upgrade (i.e., rolling > foward and back are both safe). > > > Diffs > ----- > > src/slave/containerizer/isolators/network/port_mapping.hpp > 4dd066a47d43cb1d52f93294d86309151738743e > src/slave/containerizer/isolators/network/port_mapping.cpp > 4bf0adeeac1cb6fe59f9c2ca8d5980b1500f5ddd > src/tests/port_mapping_tests.cpp 623840e71938791a562a32989775818275e6d37e > > Diff: https://reviews.apache.org/r/32426/diff/ > > > Testing > ------- > > sudo make check > > Many existing tests should already capture the regression. For example, > ROOT_CleanUpOrphanTest checks if the bind mount dir is empty after the > container is destroyed. > > Will add a compatibility test tomorrow. > > > Thanks, > > Jie Yu > >
