----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52231/#review150278 -----------------------------------------------------------
Fix it, then Ship it! src/slave/slave.cpp (lines 3622 - 3628) <https://reviews.apache.org/r/52231/#comment218176> How about checking that they're both set and they match the slave's id? ``` if (!update.has_slave_id() || update.slave_id() != info.id()) { // log warning, increment metric, return } if (!update.status().has_slave_id() || update.status().slave_id() != info.id()) { // log warning, increment metric, return } ``` As it stands it seems this check will allow the wrong slave id or unset slave ids? src/tests/command_executor_tests.cpp (lines 276 - 283) <https://reviews.apache.org/r/52231/#comment218177> Is this part necessary? - Benjamin Mahler On Sept. 24, 2016, 1:07 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52231/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2016, 1:07 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-6245 > https://issues.apache.org/jira/browse/MESOS-6245 > > > Repository: mesos > > > Description > ------- > > Previously, driver based schedulers were not able to acknowledge > status updates from HTTP based executors if they had explicit > acknowledgements enabled. This was due to the fact that we > were not populating `TaskStatus.slave_id` correctly if not > set. It would also be great to validate `TaskStatus.slave_id` > set by the executor and let them know if the value is incorrect. > > > Diffs > ----- > > src/common/protobuf_utils.cpp 5db4be4bdc7b9a3a2a66a17f8a9ac74c8d3dfbf6 > src/slave/slave.cpp 11e9c8af87aa5153f72f2a20cc578fe3d729b153 > src/tests/command_executor_tests.cpp > 07e5eb4d7c2ace2b6714fbe02f29d41663152556 > > Diff: https://reviews.apache.org/r/52231/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
