----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18551/#review35671 -----------------------------------------------------------
src/exec/exec.cpp <https://reviews.apache.org/r/18551/#comment66345> This should have been a CHECK to begin with (it's really old code), so let's just use CHECK(slave) here instead. src/exec/exec.cpp <https://reviews.apache.org/r/18551/#comment66351> Like above, this shouldn't have used 'fatal', but instead let's just use CHECK (or LOG(FATAL)) here. This is where it would be great to overload _checkSome for a Try (and Result) so we could replace this all with: CHECK_SOME(_recoveryTimeout) << "Cannot parse MESOS_RECOVERY_TIMEOUT '" << ... src/slave/slave.cpp <https://reviews.apache.org/r/18551/#comment66354> This will now capture the 'future.failure()' or 'future discarded' case, so we can just do: CHECK_READY(future) << "Failed to handle status update " << update; Given the changes in the other reviews it should print: CHECK_READY(future): is DISCARDED: Failed to handle status update ... or: CHECK_READY(future): is FAILED: failure message: Failed to handle status update ... src/slave/slave.cpp <https://reviews.apache.org/r/18551/#comment66357> In this case, we're explicitly using EXIT because we don't want to print a stack trace (or raise a SIGABRT). For problems that come from bad user input or something the user can easily rectify we don't like including the stack trace because users "freak out" rather than just reading the error message and taking the appropriate actions. We've gotten a handful of bug reports that had clear error messages that were dwarfed by stack traces, thus this approach. - Benjamin Hindman On Feb. 27, 2014, 12:30 a.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18551/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 12:30 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-1041 > https://issues.apache.org/jira/browse/MESOS-1041 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > src/exec/exec.cpp 0ab5cc20b3f86ce7f6723ef29e7a3a69d056b4ca > src/java/jni/org_apache_mesos_state_AbstractState.cpp > 0c7aebf118f7c892c61ab0c9ce92187293047f82 > src/log/leveldb.cpp 4cc39d076351df931535d7a7c84e6c805e03ffcb > src/log/log.cpp 62dc9286285d5981aa5fc63e125d3c3f51d4f457 > src/log/network.hpp 9c76bf8b2e04485da665963f104477e1324378a7 > src/log/recover.cpp 1841f1ffabe96533476621840859bef99b4abb03 > src/slave/containerizer/cgroups_launcher.cpp > a9b0108d75d0780d8c6abc82bec859ae4844c0e1 > src/slave/containerizer/launcher.cpp > 2361a20f361dc6b360770945329067de95cfd3fc > src/slave/containerizer/mesos_containerizer.cpp > 6d990cb1045bb4e68668ad0710eeb2ab5c9bbdb5 > src/slave/slave.cpp 7ad823244eb8e9a2bc3899bdf728a5faed4eafbd > > Diff: https://reviews.apache.org/r/18551/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
