----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23520/#review47910 -----------------------------------------------------------
Looks good - I added a couple of suggestions/questions and wonder how you tested it :-) src/launcher/executor.cpp <https://reviews.apache.org/r/23520/#comment84136> The health check program may initiateTaskKill without the task status being unhealthy. How about setting this variable to 'healthy' unconditionally? src/launcher/executor.cpp <https://reviews.apache.org/r/23520/#comment84137> unhealthy could be an option type, so you can encode both "killed healthy" and "killed unhealthy" src/launcher/executor.cpp <https://reviews.apache.org/r/23520/#comment84139> Maybe make this an option type? src/tests/health_check_tests.cpp <https://reviews.apache.org/r/23520/#comment84135> How did you test this if the test was disabled? - Niklas Nielsen On July 15, 2014, 1:30 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23520/ > ----------------------------------------------------------- > > (Updated July 15, 2014, 1:30 p.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Currently after the health check receives configured amount of task fails it > will initiate a kill task. > However currently the task kill status update doesn't set the healthy field > so it's unclear was it killed because health check or other reasons. > This patch sets the health check field when the task is killed because of > failing health checks. > > > Diffs > ----- > > src/launcher/executor.cpp a573637 > src/tests/health_check_tests.cpp 44711fd > > Diff: https://reviews.apache.org/r/23520/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
