----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56210/#review164584 -----------------------------------------------------------
src/launcher/executor.cpp (lines 714 - 753) <https://reviews.apache.org/r/56210/#comment236370> I think these helpers are bit confusing. For example, it's not clear to me when I should use `update()` vs `bootstrappedUpdate()` when I write new code in the future that generates a status update (say TASK_STARTING). The comment on top of `update()` seems too specific to health checks for the generic function names that you picked here. Is `update` supposed to be used when `TaskState` changes whereas the `bootstrappedUpdate` should be used when it doesn't? I hope not because even if TaskState changes you would want to preserve health and check status. Right now it kinda works because TASK_RUNNING is the only state that can have different health / check statuses, whereas TASK_KILLING or TASK_FINISHED or TASK_FINISHED don't need that info. Alternatively, can we just merge these 2 into one helper that takes a bunch of optional fields that can overwrite fields in `lastTaskStatus`? Would that be more intuitive? src/launcher/executor.cpp (line 720) <https://reviews.apache.org/r/56210/#comment236351> looks like this review doesn't use `reason` argument, so I wouldn't add it in this review. lets add it in the review that needs it. src/launcher/executor.cpp (line 752) <https://reviews.apache.org/r/56210/#comment236352> ditto. no `reason` argument in this review. src/launcher/executor.cpp (line 775) <https://reviews.apache.org/r/56210/#comment236350> s/sendTaskStatusUpdate/forward/ - Vinod Kone On Feb. 2, 2017, 9:57 a.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56210/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2017, 9:57 a.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > When a new task status update is generated in the executor, we have > to make sure specific data is duplicated from the previous update > to, e.g., avoid shadowing of those data during reconciliation. For > instance, consider a check status being sent; in this status update > we must include the latest known health information. > > > Diffs > ----- > > src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 > > Diff: https://reviews.apache.org/r/56210/diff/ > > > Testing > ------- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >
