----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29890/#review68160 -----------------------------------------------------------
Very nice refactor, Alex! RR comment: This patch touches quite a few files and could be made more consumable if you broke it up a bit further. Are you familiar with support/post-reviews.py ? General: A high-level question is what to call the default allocator and whether to move it to its own allocators directory (where it can exist along with other allocator implementations). src/master/allocator.hpp <https://reviews.apache.org/r/29890/#comment112324> This will be the public API and it would be great to have comments on all calls to be implemented. Can we leave a todo, so we get that fixed? src/master/allocator.hpp <https://reviews.apache.org/r/29890/#comment112322> Let's do this now. src/master/allocator.hpp <https://reviews.apache.org/r/29890/#comment112319> How about calling ActorAllocator the DRFAllocator or MesosAllocator (similiar to the default containerizer)? Think we could come up with a more precise name than ActorAllocator. Hopefully, other allocators are also actors :) src/master/allocator.hpp <https://reviews.apache.org/r/29890/#comment112325> Did you run into any problems making it owned? If not, can we do it now? src/tests/cluster.hpp <https://reviews.apache.org/r/29890/#comment112323> How is this different from before? src/tests/master_allocator_tests.cpp <https://reviews.apache.org/r/29890/#comment112321> So the patch will make tests flaky without a stop() method? src/tests/master_allocator_tests.cpp <https://reviews.apache.org/r/29890/#comment112320> Let us kill these. src/tests/resource_offers_tests.cpp <https://reviews.apache.org/r/29890/#comment112315> Any particular reason you removed this? - Niklas Nielsen On Jan. 14, 2015, 10:51 a.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29890/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2015, 10:51 a.m.) > > > Review request for mesos, Bernd Mathiske and Niklas Nielsen. > > > Bugs: MESOS-2213 > https://issues.apache.org/jira/browse/MESOS-2213 > > > Repository: mesos-git > > > Description > ------- > > Introduce a basic Allocator interface that every allocator should implement. > This interface does not require allocators to be based on libprocess process. > For allocators they do implement allocation logic via an internal libprocess > process, a special wrapper is provided. Allocator uses and tests are updated > to use Allocator type instead of AllocatorProcess. > > [WIP]: more refactoring will be done either in an update to this RR or in a > separate RR. > > > Diffs > ----- > > src/local/local.cpp 76e73a4 > src/master/allocator.hpp 224569a > src/master/hierarchical_allocator_process.hpp ccd37b4 > src/master/main.cpp 193d53f > src/tests/cluster.hpp 74cedb3 > src/tests/hierarchical_allocator_tests.cpp 7c05123 > src/tests/master_allocator_tests.cpp 2430622 > src/tests/mesos.hpp 591134b > src/tests/mesos.cpp 3b98c69 > src/tests/resource_offers_tests.cpp d098e70 > src/tests/slave_recovery_tests.cpp 809822e > > Diff: https://reviews.apache.org/r/29890/diff/ > > > Testing > ------- > > make distcheck (Ubuntu, OS X) > > > Thanks, > > Alexander Rukletsov > >
