> On Oct. 3, 2016, 10:28 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 4339 > > <https://reviews.apache.org/r/52470/diff/2/?file=1518346#file1518346line4339> > > > > Mutating TaskInfo should be avoided as much as possible. > > > > I don't follow why we need this. > > > > For the command executor, we were able to find it's corresponding > > executor without having to do this, why do we need it for task group tasks? > > haosdent huang wrote: > For tasks launched by the command executor, it's executor id are same > with the task id. > https://github.com/apache/mesos/blob/1.0.1/src/slave/slave.cpp#L3778 So in > Web UI, it would work that if we set the executor id to the task id when the > executor id is empty. > https://github.com/apache/mesos/blob/1.0.1/src/webui/master/static/js/controllers.js#L122 > > But for the tasks launched by the default executor, it's executor id are > given by the user > https://github.com/apache/mesos/blob/c13644984564179979d5ef5f40c8299fadd10ee7/src/cli/execute.cpp#L461-L462 > . And it may different with the task id. So when Web UI use the task id to > search the executor, it would fail.
Instead of mutating TaskInfo, lets make sure "Task.executor_id" is properly set for task group tasks in `protobuf::createTask()`. You also need to make sure default executor is added to framework and slave structs in `addTask` (this is actually a bug that we need to fix irrespective of WebUI fix: maybe file a separate ticket for that and do it in a separate review). - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52470/#review151238 ----------------------------------------------------------- On Oct. 4, 2016, 5:59 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52470/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2016, 5:59 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-6283 > https://issues.apache.org/jira/browse/MESOS-6283 > > > Repository: mesos > > > Description > ------- > > This fixed the navigate error in Web UI because Web UI uses the > executor id of the task to search the corresponding sandbox directory. > Web UI uses the task id as the executor id if the executor id of the > task is empty when searching the sandbox directory. It works fine when > tasks are launched by `CommandExecutor` because the executor id of the > task is equal to the task id in this case. However, when tasks are > launched by `DefaultExecutor`, the executor id of the task is defined > in the framework side and may different with the task id. So we need to > fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid > the Web UI uses incorrect executor id to search sandbox directory. > > > Diffs > ----- > > src/master/master.cpp 02a2fb29bdd8484fc90e5cb033ac29b49a141860 > > Diff: https://reviews.apache.org/r/52470/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
