> 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()`.
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())); ``` - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31666/#review75558 ----------------------------------------------------------- On March 3, 2015, 8:28 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31666/ > ----------------------------------------------------------- > > (Updated March 3, 2015, 8:28 p.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 dae0955d4ab29ea74ea5ab9dd6e19b028c58c6fd > src/tests/hierarchical_allocator_tests.cpp > 93753d1c04159a04a733927a487eb69505438e32 > src/tests/mesos.hpp e91e5e484eea4587ac8f2eb9cefeab4acc9f4615 > > Diff: https://reviews.apache.org/r/31666/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Michael Park > >
