> On Nov. 23, 2016, 7:28 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1400-1411 > > <https://reviews.apache.org/r/53837/diff/14/?file=1568314#file1568314line1400> > > > > hum, that means we cannot use attach with local mode? I think we should > > pass 'local' to io switchboard prepare. 'local' simply means redirect the > > container io to stdio (in fact, maybe we should have a local logger?). > > However, we should still be allowed to do attach. > > > > In local mode, we probably want to run io switchboard as an actor in > > the same linux process, rather than forking a new process. > > > > Thoughts?
For now, let's not bother with the "local" mode for the switchboard process, but I have addressed the comment abot passing locla in here for other purposes. > On Nov. 23, 2016, 7:28 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 2389-2390 > > <https://reviews.apache.org/r/53837/diff/14/?file=1568314#file1568314line2389> > > > > See my comments below. I'd suggest we don't do wait/destroy for io > > switchboard yet. As discussed offline. We are going to turn this component into an isolataor, which will remove the need for a special purpose `wait()` call. We will use the isolator `watch()` function for a similar purpose. > On Nov. 23, 2016, 7:28 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, line 168 > > <https://reviews.apache.org/r/53837/diff/14/?file=1568316#file1568316line168> > > > > Realized an FD leak in logger code. This is not yours because the > > original code has the same leak. > > > > When we return error below, we need to make sure that fds in loggerInfo > > are closed. What do you want me to do here exactly? Close the logger fds (if they are fds) if prepare() fails? I kind of feel like this should be inside the logger, not here. > On Nov. 23, 2016, 7:28 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, lines > > 337-343 > > <https://reviews.apache.org/r/53837/diff/14/?file=1568316#file1568316line337> > > > > So this is a TODO for agent restart case? What's the plan moving > > forward? Looks like previously, we don't handle logger process destory and > > wait as well. I'd suggest we don't do this optimization yet. > > > > Moving forward, do we plan to move the processes (including io > > switchboard and logger) to the container's cgroup? If that's the case, > > maybe it makes sense to make this an isolator (wrapping ioswitch board and > > logger, e.g., 'posix/io' isolator) which is always enabled (like filesystem > > or network isolator). The subprocess io info can be part of the > > ContainerLaunchInfo. If we put those processes in the same cgroup, we don't > > have to worry about destroy/wait anymore. Thoughts? As discussed offline, we are going to make this an isolator. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53837/#review156684 ----------------------------------------------------------- On Nov. 23, 2016, 2:45 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53837/ > ----------------------------------------------------------- > > (Updated Nov. 23, 2016, 2:45 a.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone. > > > Bugs: MESOS-6467 > https://issues.apache.org/jira/browse/MESOS-6467 > > > Repository: mesos > > > Description > ------- > > Currently, this process simply intercepts the stdout/stderr of a > container and writes it to the stdout/stderr FDs set up by the > container logger. We also send the stdout/stderr data to a simple HTTP > server that we launch on a unix domain socket set up by the agent. > Right now this server is just a stub and doesn't do anything useful. > > In future commits, we will expand this HTTP server to handle > 'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf > of a container. It will use the stdout/stderr messages passed to it to > and send that data over the response stream to any clients connected > with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any > input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it > to a container's stdin. > > We don't currently handle recovering access to the io switchboard > server process after agent restarts. We will add that in a subsequent > commit. > > > Diffs > ----- > > src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 > src/slave/containerizer/mesos/containerizer.cpp > e47a120bfbb607cc0cdbdaed934ae15f15666ae3 > src/slave/containerizer/mesos/io/switchboard.hpp PRE-CREATION > src/slave/containerizer/mesos/io/switchboard.cpp PRE-CREATION > src/slave/containerizer/mesos/io/switchboard_main.hpp PRE-CREATION > src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/53837/diff/ > > > Testing > ------- > > GTEST_FILTER="" make -j check > sudo src/mesos-tests > > > Thanks, > > Kevin Klues > >
