----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13620/#review25303 -----------------------------------------------------------
I like adding 'allocations', but how about some helpers in Slave and Framework too? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/13620/#comment49642> What's the difference between these two? Or why isn't 'unallocated' also "removing" the resources? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/13620/#comment49643> Pull this up to previous line? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/13620/#comment49644> What about: Slave::allocate(FrameworkID, Resources) { available -= resources; allocations[frameworkId] += resources; } and: Slave::deallocate(FrameworkID, Resources) { available += resources; allocations[frameworkId] -= resources; if (!allocations[frameworkId].allocatable()) { allocations.erase(frameworkId); } } The 'allocations.erase' part should enable you to use this code above where you remove a framework too. src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/13620/#comment49645> You could add a Framework::allocate/deallocate for consistency too: slaves[slaveId].allocate(frameworkId, resources); frameworks[frameworkId].allocate(slaveId, resources); - Benjamin Hindman On Aug. 16, 2013, 5:30 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13620/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2013, 5:30 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Bugs: MESOS-621 > https://issues.apache.org/jira/browse/MESOS-621 > > > Repository: mesos-git > > > Description > ------- > > Previously, when a slave or framework was removed the allocator didn't > recover the associated resources, instead relying on the master calling > Allocator::resourcesRecovered for all resources allocated. This was difficult > to reason about and meant that the allocator's state was sometimes > inconsistent with the reality of the cluster (for example, a framework could > have resources allocated to it on a slave that had been removed), so this > patch fixes this. > > This also solves a problem with the upcoming implementation of revocation > where resources were recovered from a removed framework and the allocator > didn't know what that framework's role is because it had been removed. > > > Diffs > ----- > > src/master/hierarchical_allocator_process.hpp 183b205 > src/master/master.cpp d53b8bb > > Diff: https://reviews.apache.org/r/13620/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Thomas Marshall > >
