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