> On March 6, 2015, 10:57 p.m., Alexander Rukletsov wrote: > > src/master/allocator/allocator.hpp, line 67 > > <https://reviews.apache.org/r/31666/diff/2/?file=883424#file883424line67> > > > > In `updateAllocation()` and `recoverResources()` we pass both `SlaveID` > > and `FrameworkID` as function parameters. How about we change the signature > > not by introducing a `hashmap` but a `SlaveID`. This will effectively move > > the loop from `addFramework()` to `Master::addFramework()`. > > Michael Park wrote: > Initially, I simply kept consistency with `addSlave`, which takes `const > hashmap<FrameworkID, Resources>& used`. > > After a bit of thought, I think it makes sense the way it is, because an > `addFramework` function should take the `FrameworkID` of the framework as > well as the total resources that framework is using. I don't think it makes > sense to report the total resources for a single framework over a sequence of > `addFramework` calls. We actually perform the following (I think perfectly > reasonable) `CHECK` inside `addFramework` as well: > > ```cpp > CHECK(!frameworkSorters[role]->contains(frameworkId.value())); > ```
Agreed. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31666/#review75558 ----------------------------------------------------------- On March 7, 2015, 10:02 a.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31666/ > ----------------------------------------------------------- > > (Updated March 7, 2015, 10:02 a.m.) > > > Review request for mesos, Alexander Rukletsov and Ben Mahler. > > > Bugs: MESOS-2373 > https://issues.apache.org/jira/browse/MESOS-2373 > > > Repository: mesos > > > Description > ------- > > Piped the master through by > `s/sum(framework->usedResources.values())/framework->usedResources/` > Lots of `s/Resources()/hashmap<SlaveID, Resources>()/` in > `src/tests/hierarchical_allocator_tests.cpp`. > > > Diffs > ----- > > src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b > src/master/allocator/mesos/allocator.hpp > fb898f1175b61b442204e6e38c69ccc2838a646f > src/master/allocator/mesos/hierarchical.hpp > c0b1da75565d9dc7728c5566f01815234163fc47 > src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec > src/tests/hierarchical_allocator_tests.cpp > 93753d1c04159a04a733927a487eb69505438e32 > src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8 > > Diff: https://reviews.apache.org/r/31666/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Michael Park > >
