Re: Review Request 71952: Set resource limits when updating executor container.

[Greg Mann <gr...@mesosphere.io>]

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




src/common/protobuf_utils.cpp
Lines 398 (patched)
<https://reviews.apache.org/r/71952/#comment308017>

    Could you move this to the patch where you add the limits field to the `Task` message?



src/slave/slave.cpp
Lines 11198 (patched)
<https://reviews.apache.org/r/71952/#comment308018>

    Seems like you could just change this entire function into a helper which we use in two places?
    
    ```
    google::protobuf::Map<string, Value::Scalar> computeExecutorLimits(
        const std::vector<TaskInfo>& tasks,
        const Resources& executorResources,
        bool enableCfs);
    ```


- Greg Mann


On Feb. 26, 2020, 12:25 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71952/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2020, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
>     https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set resource limits when updating executor container.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.cpp 7fe4a44b1e7ded998dffb0490c1d61ced697ebd5 
>   src/slave/slave.hpp 03279db082bdba23dbfeb2d93081a908e609aec2 
>   src/slave/slave.cpp cce275a504effae7a6b71dd333ce8a300d1ce5be 
> 
> 
> Diff: https://reviews.apache.org/r/71952/diff/6/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>