----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11721/#review22104 -----------------------------------------------------------
src/master/allocator.hpp <https://reviews.apache.org/r/11721/#comment45490> the naming probably predated you, but i would've preferred s/used/resources/ s/running/frameworks/ src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/11721/#comment45493> does it not already contain frameworkInfo when the slave was added? if yes, this should be a CHECK to make sure the info matches. src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/11721/#comment45491> Can you give me an example of when/how this is possible? s/dedicated/reservation/ ? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/11721/#comment45492> You probably want to do a CHECK here that 'running' contains the 'frameworkId'. src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/11721/#comment45495> How hard is it to ensure that resourceUnused() is called before frameworkRemoved()? that seems to be one of the reasons why you have to go through some of these hoops of keeping around a removed framework? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/11721/#comment45494> s/CHECK (/CHECK(/ src/master/master.cpp <https://reviews.apache.org/r/11721/#comment45496> Can you pull this down to where it is used, just before you call allocator->slaveAdded()? src/slave/slave.cpp <https://reviews.apache.org/r/11721/#comment45497> This should be outside the inner for loop. Also, do you want to send the framework info when the framework is in TERMINATING state? - Vinod Kone On June 12, 2013, 11:39 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11721/ > ----------------------------------------------------------- > > (Updated June 12, 2013, 11:39 p.m.) > > > Review request for mesos. > > > Description > ------- > > In order to make dedicated resources work, we need the allocator to always > have access to a framework's role. The way the allocator worked before, there > were two situations in which we would need to know a role but wouldn't: > > - A framework is removed at the same time as it is being allocated to, so > that a resourcesRecovered call for that framework comes after the > frameworkRemoved call. Before, we removed the framework during > frameworkRemoved and ignored the resourcesRecovered, now we keep frameworks > around after deletion until all resources have been returned from them so > that we can put those resources back into the correct pools. > > - A slave reregisters with a new master before a framework running on it > does. Previously, when the slave reconnects all we know is FrameworkIDs and > associated resources, but we can't determine how the resources on the slave > are really divided up unless we know the framework's roles. Now, the slave > sends FrameworkInfos with its reregistering message. > > > This addresses bug MESOS-505. > https://issues.apache.org/jira/browse/MESOS-505 > > > Diffs > ----- > > src/master/allocator.hpp 78c75bb > src/master/hierarchical_allocator_process.hpp 1048a28 > src/master/master.hpp 8e7b74c > src/master/master.cpp a2e4b90 > src/messages/messages.proto 2c196ee > src/slave/slave.cpp 8ce1646 > src/tests/allocator_tests.cpp 32f0a90 > src/tests/allocator_zookeeper_tests.cpp 1daaecd > src/tests/mesos.hpp fca41aa > src/tests/resource_offers_tests.cpp 3d5f02d > > Diff: https://reviews.apache.org/r/11721/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Thomas Marshall > >
