Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 20, 2015, 1:46 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Rebased. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs (updated) - src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 15, 2015, 2:17 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Rebased. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs (updated) - src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On April 2, 2015, 6:33 p.m., Vinod Kone wrote: src/Makefile.am, line 1351 https://reviews.apache.org/r/31267/diff/6/?file=912100#file912100line1351 s/drf// to be consistent (e.g., we didn't name libtestauthentication.la as libcrammd5authentication.la) Will do. On April 2, 2015, 6:33 p.m., Vinod Kone wrote: src/tests/module.cpp, line 151 https://reviews.apache.org/r/31267/diff/6/?file=912103#file912103line151 s/drf// Will do. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78694 --- On April 1, 2015, 2:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 1, 2015, 2:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 7, 2015, 12:47 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Addressed Vinod's comments. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs (updated) - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78694 --- Ship it! src/Makefile.am https://reviews.apache.org/r/31267/#comment127646 s/drf// to be consistent (e.g., we didn't name libtestauthentication.la as libcrammd5authentication.la) src/tests/module.cpp https://reviews.apache.org/r/31267/#comment127647 s/drf// - Vinod Kone On April 1, 2015, 2:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 1, 2015, 2:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 27, 2015, 4:36 p.m., Alexander Rojas wrote: src/tests/module.cpp, line 137 https://reviews.apache.org/r/31267/diff/4-5/?file=903021#file903021line137 You may need a `CHECK(modules != NULL)` or a log and return in the NULL case? Alexander Rukletsov wrote: If you follow the invokation path, you'll see that modules is a stack variable (see `initModules()`) and can't be null. I agree this is not clear at the place where we use a pointer to it and may change in the future, but this is more a general question about the design decision that should go to @tillt. Pass by pointer was introduced in Commit: 7ee3b7b672a4d8fee4fe4eb5f0aa2e7e3bf6b049 Review: https://reviews.apache.org/r/31253 Author: Till Toenshoff toensh...@me.com We generally avoid references when mutating on the callee side - that is a style descision which came from google's guidelines if I am not mistaken; see e.g. https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Parameter_Ordering - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78070 --- On March 27, 2015, 4:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 4:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 27, 2015, 4:36 p.m., Alexander Rojas wrote: src/tests/module.cpp, line 137 https://reviews.apache.org/r/31267/diff/4-5/?file=903021#file903021line137 You may need a `CHECK(modules != NULL)` or a log and return in the NULL case? If you follow the invokation path, you'll see that modules is a stack variable (see `initModules()`) and can't be null. I agree this is not clear at the place where we use a pointer to it and may change in the future, but this is more a general question about the design decision that should go to @tillt. Pass by pointer was introduced in Commit: 7ee3b7b672a4d8fee4fe4eb5f0aa2e7e3bf6b049 Review: https://reviews.apache.org/r/31253 Author: Till Toenshoff toensh...@me.com - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78070 --- On March 27, 2015, 4:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 4:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 27, 2015, 4:36 p.m., Alexander Rojas wrote: src/tests/module.cpp, line 137 https://reviews.apache.org/r/31267/diff/4-5/?file=903021#file903021line137 You may need a `CHECK(modules != NULL)` or a log and return in the NULL case? Alexander Rukletsov wrote: If you follow the invokation path, you'll see that modules is a stack variable (see `initModules()`) and can't be null. I agree this is not clear at the place where we use a pointer to it and may change in the future, but this is more a general question about the design decision that should go to @tillt. Pass by pointer was introduced in Commit: 7ee3b7b672a4d8fee4fe4eb5f0aa2e7e3bf6b049 Review: https://reviews.apache.org/r/31253 Author: Till Toenshoff toensh...@me.com Till Toenshoff wrote: We generally avoid references when mutating on the callee side - that is a style descision which came from google's guidelines if I am not mistaken; see e.g. https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Parameter_Ordering Actually the above link just menions it briefly, the one Alex just pointed me to is much more explicit: https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Reference_Arguments - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78070 --- On March 27, 2015, 4:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 4:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78509 --- Ship it! src/tests/module.cpp https://reviews.apache.org/r/31267/#comment127331 Much better than the old variant - thanks for pointing this out! - Till Toenshoff On March 27, 2015, 4:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 4:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated April 1, 2015, 2:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Addressed Alexander's comment. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs (updated) - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 3:08 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Update description. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description (updated) --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 24, 2015, 6:07 a.m., Michael Park wrote: src/tests/module.cpp, line 135 https://reviews.apache.org/r/31267/diff/4/?file=903021#file903021line135 Can we take `Modules*` here instead to keep consistency with `addAnonymousModules`, etc? or perhaps a patch before this one to make all of them `Modules`? Good catch! This is a recent change, functions signatures used to have references when I started the patch : ) On March 24, 2015, 6:07 a.m., Michael Park wrote: src/tests/module.cpp, line 141 https://reviews.apache.org/r/31267/diff/4/?file=903021#file903021line141 Looking at the `Isolator` example which has `Cpu` and `Mem` isolators, it seems to me like we want `s/testdrfalloactor/testallocator/` here. Alexander Rojas wrote: No very sure about this issue, as I see it, we can have multiple isolators, but you cannot have more than one allocator (though I may be wrong). In that sense, it makes sense to have a test for each of them. Just wondering how that will work if there are more than one allocator module that needs to be tested. Michael Park wrote: Ah, good point! I'm not sure about it either. Alex, please feel free to drop the issue. wondering how that will work if there are more than one allocator module that needs to be tested. Typed tests! Yes, I preferred clarity to consistency here. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77531 --- On March 27, 2015, 3:08 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 3:08 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 4:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Addressed MParks's comments. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs (updated) - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review78070 --- src/tests/module.cpp https://reviews.apache.org/r/31267/#comment126474 You may need a `CHECK(modules != NULL)` or a log and return in the NULL case? - Alexander Rojas On March 27, 2015, 5:26 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 27, 2015, 5:26 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. NOTE: Here we add harness code to load the module, tests will be wired up later in the patch stack. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77900 --- Can you please mention what this test do at this stage. Apparently here it only loads the module but in a later patch its functionality is actually tested. - Alexander Rojas On March 23, 2015, 3:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 3:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77903 --- Ship it! Ship It! - Alexander Rojas On March 23, 2015, 3:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 3:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 24, 2015, 7:07 a.m., Michael Park wrote: src/tests/module.cpp, line 141 https://reviews.apache.org/r/31267/diff/4/?file=903021#file903021line141 Looking at the `Isolator` example which has `Cpu` and `Mem` isolators, it seems to me like we want `s/testdrfalloactor/testallocator/` here. No very sure about this issue, as I see it, we can have multiple isolators, but you cannot have more than one allocator (though I may be wrong). In that sense, it makes sense to have a test for each of them. Just wondering how that will work if there are more than one allocator module that needs to be tested. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77531 --- On March 23, 2015, 3:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 3:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 24, 2015, 6:07 a.m., Michael Park wrote: src/tests/module.cpp, line 141 https://reviews.apache.org/r/31267/diff/4/?file=903021#file903021line141 Looking at the `Isolator` example which has `Cpu` and `Mem` isolators, it seems to me like we want `s/testdrfalloactor/testallocator/` here. Alexander Rojas wrote: No very sure about this issue, as I see it, we can have multiple isolators, but you cannot have more than one allocator (though I may be wrong). In that sense, it makes sense to have a test for each of them. Just wondering how that will work if there are more than one allocator module that needs to be tested. Ah, good point! I'm not sure about it either. Alex, please feel free to drop the issue. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77531 --- On March 23, 2015, 2:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 2:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review77531 --- src/tests/module.cpp https://reviews.apache.org/r/31267/#comment125639 Can we take `Modules*` here instead to keep consistency with `addAnonymousModules`, etc? or perhaps a patch before this one to make all of them `Modules`? src/tests/module.cpp https://reviews.apache.org/r/31267/#comment125642 Looking at the `Isolator` example which has `Cpu` and `Mem` isolators, it seems to me like we want `s/testdrfalloactor/testallocator/` here. - Michael Park On March 23, 2015, 2:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 2:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 17, 2015, 9:04 p.m., Kapil Arya wrote: src/examples/test_allocator_module.cpp, line 40 https://reviews.apache.org/r/31267/diff/3/?file=894483#file894483line40 s/HierarchicalDRFAllocator/HierarchicalDRFAllocator()/ Alexander Rukletsov wrote: There should be no difference in this case: https://stackoverflow.com/questions/620137/do-the-parentheses-after-the-type-name-make-a-difference-with-new. For consistency with instantiation on stack, let's go for original (without brackets) version. After a second thought and doing a grep over our codebase I decided to introduce parenthesis for consistency. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review76810 --- On March 13, 2015, 9:48 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 13, 2015, 9:48 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 23, 2015, 2:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Rebased. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs (updated) - src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing (updated) --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
On March 17, 2015, 9:04 p.m., Kapil Arya wrote: src/examples/test_allocator_module.cpp, line 40 https://reviews.apache.org/r/31267/diff/3/?file=894483#file894483line40 s/HierarchicalDRFAllocator/HierarchicalDRFAllocator()/ There should be no difference in this case: https://stackoverflow.com/questions/620137/do-the-parentheses-after-the-type-name-make-a-difference-with-new. For consistency with instantiation on stack, let's go for original (without brackets) version. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review76810 --- On March 13, 2015, 9:48 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 13, 2015, 9:48 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review76810 --- Ship it! LGTM minus the issue below. src/examples/test_allocator_module.cpp https://reviews.apache.org/r/31267/#comment124467 s/HierarchicalDRFAllocator/HierarchicalDRFAllocator()/ - Kapil Arya On March 13, 2015, 5:48 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 13, 2015, 5:48 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 13, 2015, 9:48 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Addressed Niklas' comments. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs (updated) - src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68 src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/#review75963 --- src/Makefile.am https://reviews.apache.org/r/31267/#comment123313 Let's keep the ordering :) src/examples/test_allocator_module.cpp https://reviews.apache.org/r/31267/#comment123314 newline src/examples/test_allocator_module.cpp https://reviews.apache.org/r/31267/#comment123315 A bit much copy past. let's kill this one here, you can leave the compatible function as NULL below. - Niklas Nielsen On March 5, 2015, 1:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 5, 2015, 1:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs - src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31267: Added a test allocator module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31267/ --- (Updated March 5, 2015, 9:05 p.m.) Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Changes --- Addressed comments. Bugs: MESOS-2160 https://issues.apache.org/jira/browse/MESOS-2160 Repository: mesos Description --- The test allocator module is a built-in HierarchicalDRFAllocator put in a module. Diffs (updated) - src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/examples/test_allocator_module.cpp PRE-CREATION src/tests/module.hpp 65e567f21744d9a2b07b8680d45bcd1ea47f9508 src/tests/module.cpp b81144f56e94034feecf3a6a4992af078cf60a81 Diff: https://reviews.apache.org/r/31267/diff/ Testing (updated) --- make check (Mac OS 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
