> On June 2, 2017, 3:39 p.m., Jan Schlicht wrote: > > src/slave/slave.cpp > > Line 5147 (original), 5147 (patched) > > <https://reviews.apache.org/r/59746/diff/1/?file=1740554#file1740554line5147> > > > > Should we also cover the `PENDING` state of a future? > > `!future.isReady()` could also mean that it's in `PENDING` state. The old > > code (wrongly) logged a "future discarded" but covered that case. > > Alexander Rukletsov wrote: > I don't think so. IIUC, this continuation is only called when a future, > on which this continuation is chained, has entered a terminal state. However, > it might make sense to `CHECK` that the future is not pending, since we > consider this an internal invariant.
Agreed, a `CHECK` makes sense here as we wouldn't expect a `PENDING` state at this point. - Jan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59746/#review176766 ----------------------------------------------------------- On June 2, 2017, 3:10 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59746/ > ----------------------------------------------------------- > > (Updated June 2, 2017, 3:10 p.m.) > > > Review request for mesos, Ian Downes, Jie Yu, Joseph Wu, and Jan Schlicht. > > > Bugs: MESOS-7601 > https://issues.apache.org/jira/browse/MESOS-7601 > > > Repository: mesos > > > Description > ------- > > Discarded future returned from the containerizer->launch() does not > necessarily mean that the container launch has failed. For example, > a framework may stop while its task are being started. > > > Diffs > ----- > > include/mesos/mesos.proto 5f80170fcd3c05add8b6e9e3107cff062818c1dc > include/mesos/v1/mesos.proto 4b528751006f709f841e44f48c9f5c2dc035b402 > src/slave/slave.cpp 0c7e5f4ef905b3897d341c3147a208fc7a8a12e0 > > > Diff: https://reviews.apache.org/r/59746/diff/1/ > > > Testing > ------- > > make check on several Linux distros. > > > Thanks, > > Alexander Rukletsov > >
