> On Dec. 10, 2014, 10:50 p.m., Jie Yu wrote: > >
Great reviews, thank you Jie! I've updated this to be a more trivial patch, which also avoids the "allocatable" issue you discovered. > On Dec. 10, 2014, 10:50 p.m., Jie Yu wrote: > > src/master/hierarchical_allocator_process.hpp, line 742 > > <https://reviews.apache.org/r/28824/diff/1/?file=785881#file785881line742> > > > > Hum, shouldn't you use foreachpair above for 'reservations' so that you > > don't need to do it here again? Alright, I've re-written it ignoring the previous structure of the code, which didn't make sense anyway given we do coarse grained allocation. :) > On Dec. 10, 2014, 10:50 p.m., Jie Yu wrote: > > src/master/hierarchical_allocator_process.hpp, line 769 > > <https://reviews.apache.org/r/28824/diff/1/?file=785881#file785881line769> > > > > You probably wanna break after that since all the reservation for that > > role on the slave have been allocated, right? Yes, re-wrote this code given that we're doing coarse-grained allocation for the foreseeable future anyway :) > On Dec. 10, 2014, 10:50 p.m., Jie Yu wrote: > > src/master/hierarchical_allocator_process.hpp, line 304 > > <https://reviews.apache.org/r/28824/diff/1/?file=785881#file785881line304> > > > > Should we add a comment here that we are assuming all used resources by > > the framework are in either star role or framework's role. It gets > > complicated once we start to introduce multiple roles for a framework and > > optimistic offer. Thanks! I've added a TODO for validating this, it will also serve as a pointer for when we come along trying to do multi-role frameworks. > On Dec. 10, 2014, 10:50 p.m., Jie Yu wrote: > > src/master/hierarchical_allocator_process.hpp, lines 434-435 > > <https://reviews.apache.org/r/28824/diff/1/?file=785881#file785881line434> > > > > Ditto. Add a note about allocated resources are in either star role or > > framework's role. Thanks, added the same TODO as above. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28824/#review64604 ----------------------------------------------------------- On Dec. 8, 2014, 10:11 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28824/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2014, 10:11 p.m.) > > > Review request for mesos, Jie Yu and Michael Park. > > > Bugs: MESOS-2176 > https://issues.apache.org/jira/browse/MESOS-2176 > > > Repository: mesos-git > > > Description > ------- > > Per MESOS-2176: > > (1) The top level role sorter always excludes reserved resources. > (2) The intra-role framework sorter always includes both reserved and > unreserved resources. > > > Diffs > ----- > > include/mesos/resources.hpp 10777a62492e4a3333d764e0f75c064694e054d1 > src/common/resources.cpp 535a0eab6377b9ae63c960cdb05978647f667d5e > src/master/hierarchical_allocator_process.hpp > f18346f435a16d1e6243315bffa00fabd164e310 > src/master/sorter.hpp aabdd0d3755be2534e2f6cae13976277ca39deb1 > src/tests/hierarchical_allocator_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/28824/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
