> On Sept. 5, 2017, 7:12 a.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/ports.cpp > > Lines 271-284 (patched) > > <https://reviews.apache.org/r/60496/diff/16/?file=1808222#file1808222line271> > > > > Mind to explain why running the loop inside the isolator process (since > > you supplied `pid` as the first parameter of `process::loop()`) and making > > `collectContainerListeners()` a static function? > > > > Can we follow the similar way of what DRF allocator process does? I > > mean we can run the loop outside the isolator process and make > > `collectContainerListeners()` a member method of > > `NetworkPortsIsolatorProcess`, and then invoke > > `collectContainerListeners()` with a `dispatch()`. In this way, we do not > > need to pass those parameters (like `portsProcess->infos.keys()`) to > > `collectContainerListeners()` since as a member method it can access them > > naturally.
> Why running the loop inside the isolator process We need to sample the updated set of container IDs (`info.keys()`), which means that at some point we need to dispatch onto the isolator process (to ensure serialized access). If we ran the `loop` on a separate process, we would need a helper method returning a `Future<vector<ContainerID>>` on the isolator that we could `dispatch` to. This would be equivalent to the code we have here. `collectContainerListeners()` is a static function because of your previous feedback that you didn't want to see a separate class to manage the additional process. > Can we follow the similar way of what DRF allocator process does? Collecting the ports is expensive (as discussed elsewhere we need to scan a large number of `/proc/fd` entries). If we `dispatch` this onto the main process, then we block the isolator process for a (potentially) long time. The very first version of this patch series implemented this suggestion and I re-wrote it because we agreed that we didn't want to block the isolator process. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60496/#review184492 ----------------------------------------------------------- On Aug. 30, 2017, 11:19 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60496/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2017, 11:19 p.m.) > > > Review request for mesos, Qian Zhang and Jiang Yan Xu. > > > Bugs: MESOS-7675 > https://issues.apache.org/jira/browse/MESOS-7675 > > > Repository: mesos > > > Description > ------- > > Implemented ports resource restrictions in the network ports isolator. > Periodically, scan for listening sockets and match them up to all > the open sockets in the containers we are tracking in the network. > Check any sockets we find against the ports resource and trigger a > resource limitation if the port has not been allocated. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/60496/diff/16/ > > > Testing > ------- > > make check (Fedora 26) > > > Thanks, > > James Peach > >
