Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94219 --- Patch looks great! Reviews applied: [37053] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 6:32 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 4, 2015, 6:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 5, 2015, 1:09 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Changes --- Added warning message if executor ids mismatch. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs (updated) - include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94277 --- Patch looks great! Reviews applied: [37053] All tests passed. - Mesos ReviewBot On Aug. 5, 2015, 5:09 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 5, 2015, 5:09 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
On Aug. 4, 2015, 11:03 p.m., Niklas Nielsen wrote: src/slave/slave.cpp, lines 2720-2721 https://reviews.apache.org/r/37053/diff/2/?file=1028868#file1028868line2720 What do you think about emitting a warning if the executor id is already set? It could be, that users have been abusing this field before (I am not aware of any, but it is a possibility). If not, we should comment on it in mesos.proto. And if it's not expected behaviour, suggest to log a warning or info message. it's better to set executor id in a central place. - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94143 --- On Aug. 4, 2015, 6:32 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 4, 2015, 6:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94143 --- Ship it! src/slave/slave.cpp (lines 2720 - 2721) https://reviews.apache.org/r/37053/#comment148658 What do you think about emitting a warning if the executor id is already set? It could be, that users have been abusing this field before (I am not aware of any, but it is a possibility). If not, we should comment on it in mesos.proto. - Niklas Nielsen On Aug. 4, 2015, 11:32 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 4, 2015, 11:32 a.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
