> On April 7, 2014, 11:57 p.m., Benjamin Hindman wrote: > > src/slave/containerizer/mesos_containerizer.cpp, line 445 > > <https://reviews.apache.org/r/20080/diff/2/?file=550552#file550552line445> > > > > For posterity I'd like you to add a comment here that explains why it's > > OK to return a Failure. In particular, we should be asking ourselves the > > question: how does the slave know that it should be sending TASK_FAILED or > > TASK_LOST for the tasks that would have been launched? In this case any > > subsequent 'wait' will return a failed future as well which the slave knows > > to treat as a "terminated" container. This makes me think that 'wait' > > should have really returned an Option<Termination> to distinguish the case > > when it doesn't know about a container ID! But that's for another review. > > Till Toenshoff wrote: > Added a verbose comment. > > Till Toenshoff wrote: > Leaving the issue open until a follow-up RR has been submitted that > introduces Option<Termination> on wait return. > > Benjamin Hindman wrote: > Okay, did you have a plan for how the external containerizer program > should return an Option<Termination>? Does that work cleanly with the > ExternalContainerizer?
Have posted a JIRA (https://issues.apache.org/jira/browse/MESOS-1243) on this for reference purposes. Will close this issue here instead. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20080/#review39749 ----------------------------------------------------------- On April 24, 2014, 9:45 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20080/ > ----------------------------------------------------------- > > (Updated April 24, 2014, 9:45 p.m.) > > > Review request for mesos, Benjamin Hindman, Ian Downes, Niklas Nielsen, and > Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Introduces the ContainerInfo protobuf as part of CommandInfo. > Right now, if present, the mesos containerizer fails the task launch to point > out that we do not support it on that containerizer. > > This will be needed for the ExternalContainerizer and possibly other > containerizers as well. > > > Diffs > ----- > > include/mesos/mesos.proto 37f8a7f > src/slave/containerizer/mesos_containerizer.cpp 4a5dfa7 > > Diff: https://reviews.apache.org/r/20080/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
