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/07 20:38:09 UTC

Review Request 13382: Modified Slave::getExecutorInfo to ensure framework_id is set.

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Repository: mesos-git


Description
-------

ExecutorInfo.framework_id is an optional field that we mark required in the proto definition.

message ExecutorInfo {
  ...
  optional FrameworkID framework_id = 8; // TODO(benh): Make this required.
  ...
}

The master does no validation to ensure this field is set correctly. As a result, our test frameworks and external frameworks are not setting this field. Even our own generated ExecutorInfo for Command Executors do not have this field set!

There are a few ways to ensure this field is set:

(1) Validate the field is set correctly in the master. This is a breaking change for existing schedulers and will require upgrade instructions to set this field when we release a version with this change.

(2) Set the field in the Master. This will be the first mutation of TaskInfo in the Master.

(3) Set the field in the SchedulerDriver. This will be the first mutation of TaskInfo in the SchedulerDriver.

(4) Slave::getExecutorInfo has a hack to generate ExecutorInfo for a Command Executor when applicable. Add another hack that sets framework_id in the ExecutorInfo when missing.

This is an implementation of (4). It's not ideal but it's a reasonable short term solution until we figure out what we'd like to do with this field.


Diffs
-----

  src/slave/slave.hpp 8ba605bafae36e0418969ba9cea51dd0cd7e91db 
  src/slave/slave.cpp 9cd7754b647dde21267f1990edb7d4e1425beacd 

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


Testing
-------

make check


Thanks,

Ben Mahler