----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54442/#review158213 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/io/switchboard.cpp (line 562) <https://reviews.apache.org/r/54442/#comment229005> This has an unfortunate race condition. If the io switchboard terminates and the pid is reused by some other process, we might be sending SIGTERM to a random process. This might be a problem under high load. Sounds to me maybe a message through domain socket is safer. You can add a TODO here and fix it later. src/slave/containerizer/mesos/io/switchboard_main.cpp (line 51) <https://reviews.apache.org/r/54442/#comment228996> write an `\n` in the end. src/slave/containerizer/mesos/io/switchboard_main.cpp (line 53) <https://reviews.apache.org/r/54442/#comment228995> User `_exit` here. `exit` is not async safe. src/slave/containerizer/mesos/io/switchboard_main.cpp (line 95) <https://reviews.apache.org/r/54442/#comment228997> Add `namespace io = process::io` above and use `io::poll` here. src/slave/containerizer/mesos/io/switchboard_main.cpp (line 97) <https://reviews.apache.org/r/54442/#comment228998> Can you handle failure case as well. Like io::poll returns a Failure or Discarded future. os::close can be moved to onAny. - Jie Yu On Dec. 6, 2016, 8:03 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54442/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2016, 8:03 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6664 > https://issues.apache.org/jira/browse/MESOS-6664 > > > Repository: mesos > > > Description > ------- > > When receiving a SIGTERM, the io switchboard process will forcibly > unblock the server from waiting on a connection before attempting to > drain its `stdoutFromFd` and `stderrFromFd` file descriptors. Once > these fds are drained (or they become invalid), the server will shut > itself down as per the normal exit route. > > > Diffs > ----- > > src/slave/containerizer/mesos/io/switchboard.cpp > 0254bd2f37cedd90f37d7b3e38a9b47e3d0fc3a6 > src/slave/containerizer/mesos/io/switchboard_main.cpp > aff6c2150bfe8086fd51b548cb6339acc23f78c9 > > Diff: https://reviews.apache.org/r/54442/diff/ > > > Testing > ------- > > make check > > The real test is encapsulated in https://reviews.apache.org/r/54367 > > > Thanks, > > Kevin Klues > >
