mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.
Date Wed, 12 Oct 2016 00:08:05 GMT

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




src/master/master.cpp (lines 3529 - 3555)
<https://reviews.apache.org/r/52470/#comment221099>

    I think we might want to mutate TaskInfo here so that authorization can use that information.
    
    More importantly, we want pending tasks to have ExecutorInfo for the operator endpoints
right? This is one of the main reasons we want to mutate TaskInfo right?
    
    ```
    const RepeatedPtrField<TaskInfo>& tasks = [&]() {
      if (operation.type() == Offer::Operation::LAUNCH) {
        return operation.launch().task_infos();
      } else if (operation.type() == Offer::Operation::LAUNCH_GROUP) {
        // We mutate TaskInfo to include ExecutorInfo to make it easy
        // for operator API and WebUI to get access to the corresponding
        // executor for task group tasks.
        for (int i = 0; i < operation.launch_group().tasks().size(); ++i) {
          TaskInfo* task = operation.launch_group().mutable_task_group()->mutable_tasks(i);
          if (!task->has_executor()) {
            task->mutable_executor()->CopyFrom(operation.launch_group().executor());
          }
        }
        
        return operation.launch_group().task_group().tasks();
      }
        UNREACHABLE();
      }();
    ```



src/master/master.cpp (line 4225)
<https://reviews.apache.org/r/52470/#comment221101>

    need to update validate to expect `task.executor()` instead of not expecting it.
    
    also need to ensure `task.executor()` is same as `launchgroup.executor` in validator.



src/master/master.cpp (line 4338)
<https://reviews.apache.org/r/52470/#comment221081>

    need to do this in `accept()` instead, as commented earlier.


- Vinod Kone


On Oct. 11, 2016, 6:06 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52470/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2016, 6:06 a.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 ad8993ae43e2508a3777f4062acaca1177ac77f2 
> 
> Diff: https://reviews.apache.org/r/52470/diff/
> 
> 
> Testing
> -------
> 
> Add a new test case `CommandExecutorTest.EmptyExecutorIdInTask`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message