You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/08/10 02:15:29 UTC

Review Request 13455: Added a check for ExecutorInfo.framework_id in the Scheduler Driver.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13455/
-----------------------------------------------------------

Review request for mesos, Benjamin Hindman and Vinod Kone.


Repository: mesos-git


Description
-------

When a Slave re-registers with the master, it sends the ExecutorInfos for running executors *and sets the framework_id in the process*.

This is problematic because the Master expects the ExecutorInfo to be immutable inside ExecutorInfoChecker. That is, any new tasks launched with the same ExecutorID will be considered invalid by the master since the slave mutated the ExecutorInfo by setting framework_id.

To fix this issue, I've modified the Scheduler Driver to set the framework_id when it is missing.

See the following review for more information: https://reviews.apache.org/r/13382/


Diffs
-----

  src/master/master.cpp a2f8929987f95e796aba71063223aea7003bf3df 
  src/sched/sched.cpp 5718fe9280d7e1124ffef2de9e4d47b347f0600a 
  src/slave/slave.cpp 3b4911844e19c85a97c79b22ef90897b26f26aac 

Diff: https://reviews.apache.org/r/13455/diff/


Testing
-------

make check


Thanks,

Ben Mahler


Re: Review Request 13455: Added a check for ExecutorInfo.framework_id in the Scheduler Driver.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13455/#review24960
-----------------------------------------------------------

Ship it!



src/sched/sched.cpp
<https://reviews.apache.org/r/13455/#comment49129>

    indentation?



src/slave/slave.cpp
<https://reviews.apache.org/r/13455/#comment49130>

    s/to set the framework id/to ensure framework id exists/


- Vinod Kone


On Aug. 10, 2013, 12:15 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13455/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2013, 12:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> When a Slave re-registers with the master, it sends the ExecutorInfos for running executors *and sets the framework_id in the process*.
> 
> This is problematic because the Master expects the ExecutorInfo to be immutable inside ExecutorInfoChecker. That is, any new tasks launched with the same ExecutorID will be considered invalid by the master since the slave mutated the ExecutorInfo by setting framework_id.
> 
> To fix this issue, I've modified the Scheduler Driver to set the framework_id when it is missing.
> 
> See the following review for more information: https://reviews.apache.org/r/13382/
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp a2f8929987f95e796aba71063223aea7003bf3df 
>   src/sched/sched.cpp 5718fe9280d7e1124ffef2de9e4d47b347f0600a 
>   src/slave/slave.cpp 3b4911844e19c85a97c79b22ef90897b26f26aac 
> 
> Diff: https://reviews.apache.org/r/13455/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13455: Ensured ExecutorInfo.framework_id is set.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13455/#review25023
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On Aug. 12, 2013, 6:32 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13455/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2013, 6:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> When a Slave re-registers with the master, it sends the ExecutorInfos for running executors *and sets the framework_id in the process*.
> 
> This is problematic because the Master expects the ExecutorInfo to be immutable inside ExecutorInfoChecker. That is, any new tasks launched with the same ExecutorID will be considered invalid by the master since the slave mutated the ExecutorInfo by setting framework_id.
> 
> To fix this issue, I've modified the Scheduler Driver to set the framework_id when it is missing.
> I've also modified the Slave to set the framework_id when generating Command Executor information.
> 
> See the following review for more information: https://reviews.apache.org/r/13382/
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp a2f8929987f95e796aba71063223aea7003bf3df 
>   src/sched/sched.cpp 5718fe9280d7e1124ffef2de9e4d47b347f0600a 
>   src/slave/slave.hpp 8ba605bafae36e0418969ba9cea51dd0cd7e91db 
>   src/slave/slave.cpp 3b4911844e19c85a97c79b22ef90897b26f26aac 
> 
> Diff: https://reviews.apache.org/r/13455/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 13455: Ensured ExecutorInfo.framework_id is set.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13455/
-----------------------------------------------------------

(Updated Aug. 12, 2013, 6:32 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Set framework_id in the slave when generating Command Executor information.


Summary (updated)
-----------------

Ensured ExecutorInfo.framework_id is set.


Repository: mesos-git


Description (updated)
-------

When a Slave re-registers with the master, it sends the ExecutorInfos for running executors *and sets the framework_id in the process*.

This is problematic because the Master expects the ExecutorInfo to be immutable inside ExecutorInfoChecker. That is, any new tasks launched with the same ExecutorID will be considered invalid by the master since the slave mutated the ExecutorInfo by setting framework_id.

To fix this issue, I've modified the Scheduler Driver to set the framework_id when it is missing.
I've also modified the Slave to set the framework_id when generating Command Executor information.

See the following review for more information: https://reviews.apache.org/r/13382/


Diffs (updated)
-----

  src/master/master.cpp a2f8929987f95e796aba71063223aea7003bf3df 
  src/sched/sched.cpp 5718fe9280d7e1124ffef2de9e4d47b347f0600a 
  src/slave/slave.hpp 8ba605bafae36e0418969ba9cea51dd0cd7e91db 
  src/slave/slave.cpp 3b4911844e19c85a97c79b22ef90897b26f26aac 

Diff: https://reviews.apache.org/r/13455/diff/


Testing
-------

make check


Thanks,

Ben Mahler


Re: Review Request 13455: Added a check for ExecutorInfo.framework_id in the Scheduler Driver.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13455/
-----------------------------------------------------------

(Updated Aug. 12, 2013, 6:06 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Updated indentation and comments.


Repository: mesos-git


Description
-------

When a Slave re-registers with the master, it sends the ExecutorInfos for running executors *and sets the framework_id in the process*.

This is problematic because the Master expects the ExecutorInfo to be immutable inside ExecutorInfoChecker. That is, any new tasks launched with the same ExecutorID will be considered invalid by the master since the slave mutated the ExecutorInfo by setting framework_id.

To fix this issue, I've modified the Scheduler Driver to set the framework_id when it is missing.

See the following review for more information: https://reviews.apache.org/r/13382/


Diffs (updated)
-----

  src/master/master.cpp a2f8929987f95e796aba71063223aea7003bf3df 
  src/sched/sched.cpp 5718fe9280d7e1124ffef2de9e4d47b347f0600a 
  src/slave/slave.cpp 3b4911844e19c85a97c79b22ef90897b26f26aac 

Diff: https://reviews.apache.org/r/13455/diff/


Testing
-------

make check


Thanks,

Ben Mahler