> On Dec. 10, 2015, 7:58 p.m., Vinod Kone wrote:
> > src/slave/qos_controllers/load.cpp, lines 236-241
> > <https://reviews.apache.org/r/40617/diff/5-6/?file=1155927#file1155927line236>
> >
> >     if you do `return`s above, this could just be else block
> >     
> >     ```
> >     } else {
> >       LOG(ERROR) << "No load thresholds are configured for 
> > LoadQoSController";
> >       return NULL;
> >     }
> >     
> >     ```

Hmm.. i think we would then miss the case when user configure any other junk 
parameter. I think we want to ignore that and end the end check if any of the 
important parameters (threshold) are set.


> On Dec. 10, 2015, 7:58 p.m., Vinod Kone wrote:
> > src/tests/oversubscription_tests.cpp, line 161
> > <https://reviews.apache.org/r/40617/diff/5-6/?file=1155928#file1155928line161>
> >
> >     it's hard to tell what this method is doing.
> >     
> >     i would rather make this a function that returns ExecutorInfo.
> >     
> >     ```
> >     ExecutorInfo createExecutorInfo(
> >         const string& executorId,
> >         const string& frameworkId
> >     )
> >     ```
> >     
> >     do you really need these executors to have different `source`s?

Nope, it is unnecessary. Killed.


- Bartek


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


On Dec. 11, 2015, 12:45 p.m., Bartek Plotka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40617/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2015, 12:45 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-4076
>     https://issues.apache.org/jira/browse/MESOS-4076
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Load QoS Controller for the simple eviction when system load is above 
> configured system load threshold for 5min and 15min:
> - Made os::loadavg called from the lambda and passed via the contructor.
> - Added unit test.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
>   src/slave/qos_controllers/load.hpp PRE-CREATION 
>   src/slave/qos_controllers/load.cpp PRE-CREATION 
>   src/tests/oversubscription_tests.cpp 
> 0333281c247dd182860a49f39be791c00679bf6b 
> 
> Diff: https://reviews.apache.org/r/40617/diff/
> 
> 
> Testing
> -------
> 
> make check
> run mesos with org_apache_mesos_LoadQoSController module included.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Reply via email to