> On Jan. 11, 2016, 11:33 a.m., Klaus Ma wrote: > > Would you add test case for that? It seems `make check` will also pass > > without this patch :). > > Guangya Liu wrote: > I think that we need to update oversubscription_tests.cpp to use > usageSlack() instead of revocable() to get usage slack resources which is > more accurate. > > Klaus Ma wrote: > That's not enough; after updating to use usageSlack(), the case in this > patch is still not covered :). I'm going to add some mix case (both > `USAGE_SLACK` & `ALLOCATION_SLACK` are reported) to catch such kind of issue. > > Guangya Liu wrote: > Does there are any cases which makes resources estimator report some > allocation slack resources?
Noop, but we need some cases to launch tasks on both `USAGE_SLACK` & `ALLOCATION_SLACK`. Current test case for Oversubscription only launch task on `USAGE_SLACK`, so no case failed. Anyway, update the related code to use `usageSlack()` make code more clear. - Klaus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42123/#review113670 ----------------------------------------------------------- On Jan. 11, 2016, 9:56 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42123/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2016, 9:56 a.m.) > > > Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van > Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu. > > > Bugs: MESOS-4322 > https://issues.apache.org/jira/browse/MESOS-4322 > > > Repository: mesos > > > Description > ------- > > The current load qos controller is calculating executor used > resources via revocable() API, but this is not right as the > revocable() will include both USAGE SLACK and ALLOCATION SLACK. > > This patch is updating the load qos controller only get USAGE > SLACK revocable resources for executors. > > The unit test was already covered in oversubscription_tests.cpp. > > > Diffs > ----- > > src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec > > Diff: https://reviews.apache.org/r/42123/diff/ > > > Testing > ------- > > make > make check > > > Thanks, > > Guangya Liu > >
