You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2012/10/25 03:58:38 UTC
Re: Review Request: Refactored Allocator
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Oct. 25, 2012, 1:58 a.m.)
Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
Changes
-------
It works!
Summary (updated)
-----------------
Refactored Allocator
Description (updated)
-------
Specifically:
--> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
--> Inside master, the allocator calls looks like method calls instead of dispatches.
Diffs (updated)
-----
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
Diff: https://reviews.apache.org/r/7720/diff/
Testing (updated)
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review13159
-----------------------------------------------------------
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment28314>
Kill.
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment28315>
Kill.
- Benjamin Hindman
On Nov. 6, 2012, 4:41 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2012, 4:41 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Refactored Allocator
>
>
> Diffs
> -----
>
> src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
> src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
> src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
> src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
> src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
> src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
> src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
> src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
> src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
> src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
> src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review13233
-----------------------------------------------------------
Ship it!
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment28464>
s/return //
src/sched/sched.cpp
<https://reviews.apache.org/r/7720/#comment28462>
s/the//
src/sched/sched.cpp
<https://reviews.apache.org/r/7720/#comment28463>
Ditto.
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/7720/#comment28461>
Kill.
src/tests/allocator_zookeeper_tests.cpp
<https://reviews.apache.org/r/7720/#comment28460>
Kill this.
src/tests/utils.hpp
<https://reviews.apache.org/r/7720/#comment28466>
It's probably time to split this out from utils.hpp, this file has too much stuff in it.
- Benjamin Hindman
On Nov. 6, 2012, 8:10 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2012, 8:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Refactored Allocator.
>
> Features:
>
> --> Master can make direct method calls on the Allocator instead of doing dipatches.
>
> --> MockAllocatorProcess does dispatches to the underlying allocator process instead of directly invoking the methods.
>
>
> Diffs
> -----
>
> src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
> src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
> src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
> src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
> src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
> src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
> src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
> src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
> src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
> src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
> src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review13202
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On Nov. 6, 2012, 8:10 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2012, 8:10 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Refactored Allocator.
>
> Features:
>
> --> Master can make direct method calls on the Allocator instead of doing dipatches.
>
> --> MockAllocatorProcess does dispatches to the underlying allocator process instead of directly invoking the methods.
>
>
> Diffs
> -----
>
> src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
> src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
> src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
> src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
> src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
> src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
> src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
> src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
> src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
> src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
> src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Nov. 6, 2012, 8:10 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
posting the proper diff.
please look at the diff between diffs 6-8 to see the changes.
Description
-------
Refactored Allocator.
Features:
--> Master can make direct method calls on the Allocator instead of doing dipatches.
--> MockAllocatorProcess does dispatches to the underlying allocator process instead of directly invoking the methods.
Diffs (updated)
-----
src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
Diff: https://reviews.apache.org/r/7720/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Nov. 6, 2012, 8:08 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Reverted Allocator methods to return void.
Formatting.
Description (updated)
-------
Refactored Allocator.
Features:
--> Master can make direct method calls on the Allocator instead of doing dipatches.
--> MockAllocatorProcess does dispatches to the underlying allocator process instead of directly invoking the methods.
Diffs (updated)
-----
Makefile.am cf287fc129fe17ebf31812026d0b8f08067fb2bf
src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
src/detector/detector.hpp d859b080b99e23d511458a27272db33c5486bb4b
src/detector/detector.cpp 62df8bdf539eb13b2a6dc00eb2f6a07381d59106
src/examples/balloon_executor.cpp 9f3783b24427d3102c89b1c843fc70c842156395
src/linux/cgroups.hpp 8147919bf7f17e50e047f813db58f99fb9dfffe5
src/linux/cgroups.cpp a6056c31d504f5570872c0ed123b28400640d5cf
src/linux/proc.hpp dd0b30ca07c7bfb89d008f9b7a4441491fbae7fd
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
src/slave/cgroups_isolation_module.hpp 76d916f8882b4e116327ed3249b01d439b5b89d5
src/slave/cgroups_isolation_module.cpp a43d31760f14e641cc7d75b6e98041f98bba2b63
src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d
src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e
src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
src/tests/balloon_framework_test.sh 7cbd0749011970184158e3bb6c28820d47613327
src/tests/cgroups_tests.cpp 49d835c472f6acf8bff268b0370bedfd13eb555a
src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130
src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
src/tests/zookeeper_test_server.hpp 06320439b993f9612ea01303f7446dadf97dc045
src/tests/zookeeper_tests.cpp 3f001affe0dd4b8002e99a658c47b8ea86ddb7d6
support/atexit.sh PRE-CREATION
support/colors.sh d679da19bd8d7b946d18df1af97f8d6075157d53
Diff: https://reviews.apache.org/r/7720/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Nov. 6, 2012, 4:41 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Fixed MockAllocatorProcess to dispatch to the underlying allocator process.
Description
-------
Refactored Allocator
Diffs (updated)
-----
src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
Diff: https://reviews.apache.org/r/7720/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Oct. 30, 2012, 12:06 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off trunk
Description (updated)
-------
Refactored Allocator
Diffs (updated)
-----
src/Makefile.am 062c45c060c67e983d0c27770eb3f1eefd582c8f
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193
src/master/master.hpp 1d8d0e4b0080d5efb25f8140e4a9bafdff513469
src/master/master.cpp 8b6c71575d57816588a52dfc0fa74e51df1bea4d
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 6f190e9f86d2ecdcfed8ec06831940606332e536
src/tests/allocator_tests.cpp b0056b983b3a604e9124306cb12a366e5896a588
src/tests/allocator_zookeeper_tests.cpp 72ec292cf5a85d2012d940a347293ef105206ef1
src/tests/fault_tolerance_tests.cpp a01d1aef012b636f2ced64d4d2ffabfb6ce42644
src/tests/gc_tests.cpp b61b2de621e227f327ce546b62f8dfc528f3894e
src/tests/master_detector_tests.cpp c0dc301f2a9f8d586864794c0526caac6c68727e
src/tests/master_tests.cpp 7112c8c877eb4d852c36d65a7d44af74652239b3
src/tests/resource_offers_tests.cpp 9f7916873423382443f39c31b1d682dc0e4071bb
src/tests/utils.hpp e0036c04bef60d62cc4314a7e0beb02a0071a808
Diff: https://reviews.apache.org/r/7720/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/
-----------------------------------------------------------
(Updated Oct. 29, 2012, 2:16 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Ben's comments.
I'm getting a very strange* SEGFAULT issue, when I move allocator.cpp into allocator.hpp and inline all the methods.
The first test succeeds, and then second one segfaults. Here is a gdb trace.
[ OK ] MasterTest.TaskRunning (99 ms)
[ RUN ] MasterTest.KillTask
I1028 13:32:40.872483 28352512 master.cpp:304] Master started on 192.168.1.127:57593
I1028 13:32:40.872520 28352512 master.cpp:319] Master ID: 201210281332-2130815168-57593-19716
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x00000002ffffffa0
[Switching to process 19716]
0x00007fff81a61162 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string ()
(gdb) thread apply all bt
Thread 6 (process 19716):
#0 0x00007fff8a4f6932 in select$DARWIN_EXTSN ()
#1 0x0000000100eca3d8 in select_poll (loop=0x10111cea0, timeout=<value temporarily unavailable, due to optimizations>) at ev_select.c:168
#2 0x0000000100ecb1e7 in ev_loop (loop=0x10111cea0, flags=<value temporarily unavailable, due to optimizations>) at ev.c:2311
#3 0x0000000100d5996e in process::serve () at basic_string.h:238
#4 0x00007fff8a4ebfd6 in _pthread_start ()
#5 0x00007fff8a4ebe89 in thread_start ()
Thread 5 (process 19716):
#0 0x00007fff81a61162 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string ()
#1 0x00000001000d7a0e in Flag [inlined] () at /Users/vinod/workspace/apache/mesos/src/flags/flag.hpp:107
#2 Flag [inlined] () at flag.hpp:69
#3 pair [inlined] () at /usr/include/c++/4.2.1/bits/stl_pair.h:14
#4 pair [inlined] () at flag.hpp:107
#5 __gnu_cxx::new_allocator<std::pair<std::string const, flags::Flag> >::construct () at /usr/include/c++/4.2.1/ext/new_allocator.h:69
#6 std::_Rb_tree<std::string, std::pair<std::string const, flags::Flag>, std::_Select1st<std::pair<std::string const, flags::Flag> >, std::less<std::string>, std::allocator<std::pair<std::string const, flags::Flag> > >::_M_create_node () at /usr/include/c++/4.2.1/bits/stl_tree.h:380
#7 std::_Rb_tree<std::string, std::pair<std::string const, flags::Flag>, std::_Select1st<std::pair<std::string const, flags::Flag> >, std::less<std::string>, std::allocator<std::pair<std::string const, flags::Flag> > >::_M_clone_node () at /usr/include/c++/4.2.1/bits/stl_tree.h:392
#8 0x00000001000d7a0e in std::_Rb_tree<std::string, std::pair<std::string const, flags::Flag>, std::_Select1st<std::pair<std::string const, flags::Flag> >, std::less<std::string>, std::allocator<std::pair<std::string const, flags::Flag> > >::_M_copy (this=0x101b08cc0, __x=0x2ffffff80, __p=0x101b08cc8) at stl_pair.h:1287
#9 0x0000000100bcb0bf in process::dispatch<mesos::internal::master::AllocatorProcess, mesos::internal::master::Flags const&, process::PID<mesos::internal::master::Master> const&, mesos::internal::master::Flags, process::PID<mesos::internal::master::Master> > () at basic_string.h:238
#10 0x0000000100b864d6 in mesos::internal::master::Master::initialize () at basic_string.h:238
#11 0x0000000100d5daa0 in process::ProcessManager::resume () at basic_string.h:238
#12 0x0000000100d5e47c in process::schedule () at basic_string.h:238
#13 0x00007fff8a4ebfd6 in _pthread_start ()
#14 0x00007fff8a4ebe89 in thread_start ()
Thread 4 (process 19716):
#0 0x00007fff8a4eda6a in __semwait_signal ()
#1 0x00007fff8a4f1881 in _pthread_cond_wait ()
#2 0x0000000100db0d10 in Gate::arrive () at basic_string.h:238
#3 0x0000000100d5e45e in process::schedule () at basic_string.h:238
#4 0x00007fff8a4ebfd6 in _pthread_start ()
#5 0x00007fff8a4ebe89 in thread_start ()
Thread 3 (process 19716):
#0 0x00007fff8a4eda6a in __semwait_signal ()
#1 0x00007fff8a4f1881 in _pthread_cond_wait ()
#2 0x0000000100db0d10 in Gate::arrive () at basic_string.h:238
#3 0x0000000100d5e45e in process::schedule () at basic_string.h:238
#4 0x00007fff8a4ebfd6 in _pthread_start ()
#5 0x00007fff8a4ebe89 in thread_start ()
Thread 2 (process 19716):
#0 0x00007fff8a4eda6a in __semwait_signal ()
#1 0x00007fff8a4f1881 in _pthread_cond_wait ()
#2 0x0000000100db0d10 in Gate::arrive () at basic_string.h:238
#3 0x0000000100d5e45e in process::schedule () at basic_string.h:238
#4 0x00007fff8a4ebfd6 in _pthread_start ()
#5 0x00007fff8a4ebe89 in thread_start ()
Thread 1 (process 19716):
#0 MasterTest_KillTask_Test::TestBody (this=<value temporarily unavailable, due to optimizations>) at stl_vector.h:608
#1 0x000000010032fe05 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void> () at basic_string.h:493
#2 0x000000010033ae6f in testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void> () at basic_string.h:493
#3 0x00000001003274d1 in testing::Test::Run () at basic_string.h:493
#4 0x000000010032e034 in testing::TestInfo::Run () at basic_string.h:493
#5 0x000000010032e187 in testing::TestCase::Run () at basic_string.h:493
#6 0x000000010032e476 in testing::internal::UnitTestImpl::RunAllTests () at basic_string.h:493
#7 0x00000001003302b7 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> () at basic_string.h:493
#8 0x000000010033a8f8 in testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> () at basic_string.h:493
#9 0x0000000100326c81 in testing::UnitTest::Run () at basic_string.h:493
#10 0x0000000100019b92 in main (argc=2, argv=<value temporarily unavailable, due to optimizations>) at ../../src/tests/main.cpp:205
Description
-------
Specifically:
--> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
--> Inside master, the allocator calls looks like method calls instead of dispatches.
Diffs (updated)
-----
src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
Diff: https://reviews.apache.org/r/7720/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 26, 2012, 6:27 p.m., Ben Mahler wrote:
> > Is this the latest diff?
no. i will upload it once i get my tests to pass.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review12833
-----------------------------------------------------------
On Oct. 25, 2012, 1:58 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
>
>
> Description
> -------
>
> Specifically:
>
> --> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
>
> --> Inside master, the allocator calls looks like method calls instead of dispatches.
>
>
> Diffs
> -----
>
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
> src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
> src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
> src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
> src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
> src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
> src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
> src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
> src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
> src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
> src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review12833
-----------------------------------------------------------
Is this the latest diff?
- Ben Mahler
On Oct. 25, 2012, 1:58 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
>
>
> Description
> -------
>
> Specifically:
>
> --> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
>
> --> Inside master, the allocator calls looks like method calls instead of dispatches.
>
>
> Diffs
> -----
>
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
> src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
> src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
> src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
> src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
> src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
> src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
> src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
> src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
> src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
> src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/allocator.hpp, line 39
> > <https://reviews.apache.org/r/7720/diff/3/?file=179757#file179757line39>
> >
> > Why not just put this forward declaration down in in the mesos::internal block below?
It felt a bit ugly to stick it below. But, opening and closing namespace for fwd declarations is also not pretty.
i'm fine either way. Lets stick with one style. I will follow the former style for now.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/allocator.hpp, line 128
> > <https://reviews.apache.org/r/7720/diff/3/?file=179757#file179757line128>
> >
> > Do you want people to extend Allocator?
No, dropped virtual. But, that's not enough to stop inheritance right? I need a public factory method and make the constructor private.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/allocator.hpp, line 170
> > <https://reviews.apache.org/r/7720/diff/3/?file=179757#file179757line170>
> >
> > This isn't necessary because you defined a constructor above. But what about copying and assigning?
done
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/allocator.cpp, line 29
> > <https://reviews.apache.org/r/7720/diff/3/?file=179758#file179758line29>
> >
> > If we include all of this implementation in the bottom of allocator.hpp than we will get the dispatch calls inlined, effectively giving us the exact same performance characteristics as before but with the much nicer syntax. Sound good!?
as discussed offline, this is a trade-off between possible increase in compilation time vs run-time performance. but, i do agree that this file is less likely to be touched in the future (famous last words?) and hence am ok with pulling the impl up into the hpp.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/allocator.cpp, line 145
> > <https://reviews.apache.org/r/7720/diff/3/?file=179758#file179758line145>
> >
> > So who deletes the HierarchicalAllocatorProcess instance?
good point.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/master/hierarchical_allocator_process.hpp, line 490
> > <https://reviews.apache.org/r/7720/diff/3/?file=179761#file179761line490>
> >
> > Eh, it looked less jagged before; I was quite happy with how Thomas wrapped these.
done
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 304
> > <https://reviews.apache.org/r/7720/diff/3/?file=179767#file179767line304>
> >
> > I think when we use 'DoAll' it's easier to read each thing that we're doing by wrapping, so:
> >
> > Trigger(...).
> > Return(Nothing)
agreed, done. here and everywhere else in this file!
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 836
> > <https://reviews.apache.org/r/7720/diff/3/?file=179767#file179767line836>
> >
> > Why 6.0?
my bad, forgot to remove it after my debugging session.
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_zookeeper_tests.cpp, line 153
> > <https://reviews.apache.org/r/7720/diff/3/?file=179768#file179768line153>
> >
> > This is going to conflict with my pending review.
i copied it based on your review :)
> On Oct. 25, 2012, 4:53 p.m., Benjamin Hindman wrote:
> > src/tests/utils.hpp, line 258
> > <https://reviews.apache.org/r/7720/diff/3/?file=179774#file179774line258>
> >
> > Why do you need to do this? Provided that they were dispatched to MockAllocatorProcess directly invoking should be fine. Since this is a process, any direct calls to it should be forbidden anyway.
In general, I dont like the fact that we are directly making calls to a libprocess process (real) from outside it (from MockAllocatorProcess). The real process is/might've been written based on the assumption that its public functions are called via dispatch. In other words, no two public functions would be expected to be executed at the same time, or no two external entities can be executing the same function at the same time. Does that make sense?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review12764
-----------------------------------------------------------
On Oct. 25, 2012, 1:58 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
>
>
> Description
> -------
>
> Specifically:
>
> --> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
>
> --> Inside master, the allocator calls looks like method calls instead of dispatches.
>
>
> Diffs
> -----
>
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
> src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
> src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
> src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
> src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
> src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
> src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
> src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
> src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
> src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
> src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review12764
-----------------------------------------------------------
Ship it!
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27290>
Why not just put this forward declaration down in in the mesos::internal block below?
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27289>
Not in headers please, even though this is scoped within namespaces.
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27291>
I liked the formatting before better, I think it's more readable (much less whitespace, much less jaggedness).
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27292>
Do you want people to extend Allocator?
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27293>
This isn't necessary because you defined a constructor above. But what about copying and assigning?
src/master/allocator.hpp
<https://reviews.apache.org/r/7720/#comment27294>
s/allocatorProcess/process/
src/master/allocator.cpp
<https://reviews.apache.org/r/7720/#comment27314>
If we include all of this implementation in the bottom of allocator.hpp than we will get the dispatch calls inlined, effectively giving us the exact same performance characteristics as before but with the much nicer syntax. Sound good!?
src/master/allocator.cpp
<https://reviews.apache.org/r/7720/#comment27296>
I don't think this "factory" is needed right now (or maybe ever). Please kill (see comment below for why).
src/master/allocator.cpp
<https://reviews.apache.org/r/7720/#comment27295>
So who deletes the HierarchicalAllocatorProcess instance?
src/master/drf_sorter.cpp
<https://reviews.apache.org/r/7720/#comment27297>
I've just been including "logging/logging.hpp" so as we add new abstractions we only have to include one thing (and then everything is consistent).
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/7720/#comment27299>
Please wrap 'resources' then too.
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/7720/#comment27301>
Eh, it looked less jagged before; I was quite happy with how Thomas wrapped these.
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/7720/#comment27300>
Ditto above comment.
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/7720/#comment27302>
Ditto comment above.
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/7720/#comment27303>
Ditto comment above.
src/master/master.hpp
<https://reviews.apache.org/r/7720/#comment27304>
Can kill newline.
src/master/master.cpp
<https://reviews.apache.org/r/7720/#comment27305>
Pretty!
src/master/master.cpp
<https://reviews.apache.org/r/7720/#comment27306>
Oooh!
src/master/master.cpp
<https://reviews.apache.org/r/7720/#comment27307>
Aaaaahhhh!
src/master/master.cpp
<https://reviews.apache.org/r/7720/#comment27308>
Okay, I'll stop now. ;)
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/7720/#comment27309>
I think when we use 'DoAll' it's easier to read each thing that we're doing by wrapping, so:
Trigger(...).
Return(Nothing)
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/7720/#comment27310>
Why 6.0?
src/tests/allocator_zookeeper_tests.cpp
<https://reviews.apache.org/r/7720/#comment27311>
This is going to conflict with my pending review.
src/tests/allocator_zookeeper_tests.cpp
<https://reviews.apache.org/r/7720/#comment27312>
These too. :(
src/tests/utils.hpp
<https://reviews.apache.org/r/7720/#comment27313>
Why do you need to do this? Provided that they were dispatched to MockAllocatorProcess directly invoking should be fine. Since this is a process, any direct calls to it should be forbidden anyway.
- Benjamin Hindman
On Oct. 25, 2012, 1:58 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
>
>
> Description
> -------
>
> Specifically:
>
> --> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
>
> --> Inside master, the allocator calls looks like method calls instead of dispatches.
>
>
> Diffs
> -----
>
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
> src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
> src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
> src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
> src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
> src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
> src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
> src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
> src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
> src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
> src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored Allocator
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7720/#review12767
-----------------------------------------------------------
src/local/local.hpp
<https://reviews.apache.org/r/7720/#comment27315>
Move these declarations below so that there are less namespaces opened and closed.
- Benjamin Hindman
On Oct. 25, 2012, 1:58 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7720/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Thomas Marshall, and Ben Mahler.
>
>
> Description
> -------
>
> Specifically:
>
> --> Allocator writers don't need to do extra work to make their allocator async. This is guaranteed/enforced by the Allocator interface.
>
> --> Inside master, the allocator calls looks like method calls instead of dispatches.
>
>
> Diffs
> -----
>
> src/local/local.hpp cd0483c72f919e08a67b4eeaebef3b62b0ff9798
> src/local/local.cpp 67c7a7c260d2ea610fb64b491f9bf7a22ec53aac
> src/master/allocator.hpp 9fd790564d1361b9bdb2111f29348a631f66c831
> src/master/allocator.cpp 3f822912a4e0c50c1612eac98ae85447aae023d4
> src/master/drf_sorter.hpp 9c43ba32c6ed04fd6960524b5925c5c27c3e4d2d
> src/master/drf_sorter.cpp 0ab4a14ca0bfb721de00e29d993fd4aa08a27ac5
> src/master/hierarchical_allocator_process.hpp 266d339bdb72f8c63288d91c1f514da07ee9acf2
> src/master/main.cpp 813ef7de8e57b1cbd688e6e34e360c7b191ed641
> src/master/master.hpp 146af017bbb6da9bd44acb53a4f1ee0ffbedd64f
> src/master/master.cpp 82e4dc704e5c67ec178bd058934896328308d868
> src/master/sorter.hpp 80d0893b1f988898e40ae3a2f6f19a117ad5f72d
> src/sched/sched.cpp 5a4e594a3a8a2089cf74e7e418f2e7bde4c03e9d
> src/tests/allocator_tests.cpp ed52ff61b1820cb3f5f7f2ae421fb3f84a85939a
> src/tests/allocator_zookeeper_tests.cpp bd408c22df5edc3684791e509ce1a4c210553922
> src/tests/fault_tolerance_tests.cpp 558bf41ab6f4ba319ec3b23f3347f98e235260f9
> src/tests/gc_tests.cpp 1de240579a1cc6af8738d2ffb5da1b3e01c4b19e
> src/tests/master_detector_tests.cpp fb32af8130d3f359eecba5389c2e42c247d6b70e
> src/tests/master_tests.cpp 37e97e0a58a0128579f155183b7e769ab4e1b452
> src/tests/resource_offers_tests.cpp c50da38508694f956ad104fb160896eb9b9d5fea
> src/tests/utils.hpp 4066c02b23db15310104955465c4e4afd7bcad5b
>
> Diff: https://reviews.apache.org/r/7720/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>