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 Thu, 05 Mar 2020 15:44:04 GMT


> On Feb. 28, 2020, 5:25 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 11198 (patched)
> > <https://reviews.apache.org/r/71952/diff/6/?file=2212191#file2212191line11198>
> >
> >     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?

See my comment below regarding changing this method into a function helper.


- Greg


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


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