----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29556/#review67110 -----------------------------------------------------------
src/common/protobuf_utils.cpp <https://reviews.apache.org/r/29556/#comment110933> Whoops, please put the brace on the next line! Also, could we call this `getTaskHealth` or at least `latestTaskHealth`? src/common/protobuf_utils.cpp <https://reviews.apache.org/r/29556/#comment110935> Mind removing this variable? src/tests/health_check_tests.cpp <https://reviews.apache.org/r/29556/#comment110936> Mind adding a newline here for readability? - Ben Mahler On Jan. 6, 2015, 7:28 a.m., Connor Doyle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29556/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2015, 7:28 a.m.) > > > Review request for mesos, Adam B and Timothy Chen. > > > Bugs: MESOS-2206 > https://issues.apache.org/jira/browse/MESOS-2206 > > > Repository: mesos-git > > > Description > ------- > > Task statuses sent to the framework scheduler from the master do not include > a value for the `healthy` field. The result is that the scheduler is left > with incomplete knowledge about the health state of running tasks > indefinitely, until by chance the executor sends a new status update. > > - Update the createTaskStatus auxilliary function in protobuf utils to take > an optional health flag. > - Pass known health to createTaskStatus during implicit and explicit task > reconciliation. > > > Diffs > ----- > > src/common/protobuf_utils.hpp e42aaa5 > src/common/protobuf_utils.cpp 8ab5cdd > src/master/master.cpp d6651e2 > src/tests/health_check_tests.cpp a707398 > > Diff: https://reviews.apache.org/r/29556/diff/ > > > Testing > ------- > > - make > - make check > > Modified one of the health check tests to ensure that the TaskStatus induced > by implicit and explicit reconciliation have the health field set properly. > > Manual testing using a development branch of the Marathon[1] framework: > > - Launched two tasks with health checks. > - Waited for both tasks to enter the running state. > - Waited for both tasks to report healthy. > - Shut down the scheduler. > - Re-started (re-registered) the scheduler. > - Observed indeterminate health state for both tasks as reported by the > framework. > - Waited for the framework to initiate task reconciliation. > - Observed a TaskStatus with the healthy field set to `true` for both tasks. > > [1] http://github.com/mesosphere/marathon > > > Thanks, > > Connor Doyle > >
