mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 71952: Set resource limits when updating executor container.
Date Fri, 28 Feb 2020 17:25:10 GMT

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




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

    The similarity of this function body to some of the code in https://reviews.apache.org/r/71858/
is unfortunate... do you think we should factor it out into some helper which looks through
a list of tasks and calculates the limits and requests? Maybe something like:
    
    ```
    struct TaskResourceMetadata
    {
      Resources totalTaskResources;
      map<string, Option<Value::Scalar>> totalLimits;
      map<string, Value::Scalar> totalRequests;
    };
    
    TaskResourceMetadata computeTaskResourceMetadata(vector<TaskInfo> tasks)
    {
      TaskResourceMetadata result;
      std::vector<std::string> names({"cpus", "mem"});
    
      foreach (const TaskInfo& _task, tasks) {
        result.totalTaskResources += _task.resources();
    
        foreach (const std::string _name, names) {
          // Lazily initialize requests and limits with `operator[]`.
          if (_task.limits().count(_name)) {
            setLimit(result.totalLimits[_name], _task.limits().at(_name));
          } else {
            Option<Value::Scalar> taskRequest =
              Resources(_task.resources()).get<Value::Scalar>(_name);
    
            if (taskRequest.isSome()) {
              result.totalRequests[_name] += taskRequest.get();
            }
          }
        }
      }
    
      return result;
    }
    ```
    
    WDYT? Is it worth it?


- 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
> 
>


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