You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Bartek Plotka <bw...@gmail.com> on 2015/05/26 05:55:35 UTC
Review Request 34662: Modularized ResourceEstimator and added test
for that module
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
src/tests/resource_estimator_module.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for that module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 26, 2015, 4:16 a.m., Szymon Konefal wrote:
> > src/tests/oversubscription_tests.cpp, line 80
> > <https://reviews.apache.org/r/34662/diff/1/?file=971532#file971532line80>
> >
> > Why CHECK_SOME rather than ASSERT_SOME?
> > CHECK_SOME prints only a log, and ASSERT_SOME could return ::testing::AssertionFailure()
Also was not sure about it, but i was inspired by that:
(*isolator_tests.cpp:152*)
```
Try<Isolator*> isolator = TypeParam::create(flags);
CHECK_SOME(isolator);
```
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85146
-----------------------------------------------------------
On May 26, 2015, 4:36 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 26, 2015, 4:36 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
> src/tests/resource_estimator_module.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for that module
Posted by Szymon Konefal <sz...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85146
-----------------------------------------------------------
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment136634>
Why CHECK_SOME rather than ASSERT_SOME?
CHECK_SOME prints only a log, and ASSERT_SOME could return ::testing::AssertionFailure()
- Szymon Konefal
On May 26, 2015, 3:55 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 26, 2015, 3:55 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
> src/tests/resource_estimator_module.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 27, 2015, 12:57 a.m., Niklas Nielsen wrote:
> > src/tests/resource_estimator.hpp, line 44
> > <https://reviews.apache.org/r/34662/diff/3/?file=972380#file972380line44>
> >
> > We usually stick '&' and '*' with the type, not the name :)
> >
> > So it should be:
> >
> > const Foo& bar;
> >
> > Here and below
All of that was modified automatically by my clion.. customized the options now to the mesos style (:
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85304
-----------------------------------------------------------
On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 1:34 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85304
-----------------------------------------------------------
Small nits - we are close :)
src/Makefile.am
<https://reviews.apache.org/r/34662/#comment136844>
Is the alignment off?
src/examples/test_resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment136852>
newline
src/tests/module.cpp
<https://reviews.apache.org/r/34662/#comment136851>
Mind wrapping like:
addModule(
library, TestResourceEstimator, "org_apache_mesos_TestResourceEstimator");
(Wrap at start, 4 space indent) instead? Try to take a look at our style guide for wrapping rules :)
See line 138 too
src/tests/module.cpp
<https://reviews.apache.org/r/34662/#comment136850>
s/Test//
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136849>
newline below
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136848>
newline below
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136845>
We usually stick '&' and '*' with the type, not the name :)
So it should be:
const Foo& bar;
Here and below
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136847>
s/<ResourceEstimator */<ResourceEstimator*/
- Niklas Nielsen
On May 26, 2015, 5:15 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 26, 2015, 5:15 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
> On May 27, 2015, 5:24 p.m., Vinod Kone wrote:
> > src/examples/test_resource_estimator_module.cpp, lines 40-44
> > <https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40>
> >
> > I'm confused. 'parameters' are unused and the passed 'flags' are empty?
TestResourceEstimator need to have a 'create()' which takes a Flags object, to be consistent with modules typed tests; so it's is just to avoid another 'create()'. Would you prefer that?
Parameters could be removed - @Bartek Plotka, mind removing that one? :)
> On May 27, 2015, 5:24 p.m., Vinod Kone wrote:
> > src/tests/resource_estimator.hpp, line 47
> > <https://reviews.apache.org/r/34662/diff/6/?file=973217#file973217line47>
> >
> > s/consistent/to be consistent/
Should be fixed.
> On May 27, 2015, 5:24 p.m., Vinod Kone wrote:
> > src/tests/resource_estimator.hpp, lines 55-57
> > <https://reviews.apache.org/r/34662/diff/6/?file=973217#file973217line55>
> >
> > Hmm. I'm confused. Why the change in semantics here?
The typed tests for modules will give you a ResourceEstimator - not a TestResourceEstimator. Therefore, we cannot have the 'estimate()' convenience method on the estimator. Does this make sense?
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 27, 2015, 5:04 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 5:04 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 28, 2015, 12:24 a.m., Vinod Kone wrote:
> > src/examples/test_resource_estimator_module.cpp, lines 40-44
> > <https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40>
> >
> > I'm confused. 'parameters' are unused and the passed 'flags' are empty?
>
> Niklas Nielsen wrote:
> TestResourceEstimator need to have a 'create()' which takes a Flags object, to be consistent with modules typed tests; so it's is just to avoid another 'create()'. Would you prefer that?
>
> Parameters could be removed - @Bartek Plotka, mind removing that one? :)
>
> Bartek Plotka wrote:
> sure (:
>
> Bartek Plotka wrote:
> However we would need to create new interface method for Module if we don't want use parameters in that case...
@vinod and @niklas please see test_allocator_module.cpp it's quite common because of the convention. Even that parameters are not used.
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 28, 2015, 1:20 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 1:20 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 28, 2015, 12:24 a.m., Vinod Kone wrote:
> > src/examples/test_resource_estimator_module.cpp, lines 40-44
> > <https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40>
> >
> > I'm confused. 'parameters' are unused and the passed 'flags' are empty?
>
> Niklas Nielsen wrote:
> TestResourceEstimator need to have a 'create()' which takes a Flags object, to be consistent with modules typed tests; so it's is just to avoid another 'create()'. Would you prefer that?
>
> Parameters could be removed - @Bartek Plotka, mind removing that one? :)
>
> Bartek Plotka wrote:
> sure (:
However we would need to create new interface method for Module if we don't want use parameters in that case...
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 28, 2015, 12:04 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 12:04 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 28, 2015, 12:24 a.m., Vinod Kone wrote:
> > src/tests/resource_estimator.hpp, line 39
> > <https://reviews.apache.org/r/34662/diff/6/?file=973217#file973217line39>
> >
> > How come we didn't have this problem with TestAllocator?
Because there was no need for TestAllocator to make a seperate class for the unit tests purpose only.
While for ResourceEstimator it is necessary - we need to activate something inside it to enforce that RE will send slack resources to slave and make a meaningful test.
It'll be more clear when we pass ResourceUsage to ResourceEstimator (:
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 28, 2015, 12:04 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 12:04 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 28, 2015, 12:24 a.m., Vinod Kone wrote:
> > src/examples/test_resource_estimator_module.cpp, lines 40-44
> > <https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40>
> >
> > I'm confused. 'parameters' are unused and the passed 'flags' are empty?
>
> Niklas Nielsen wrote:
> TestResourceEstimator need to have a 'create()' which takes a Flags object, to be consistent with modules typed tests; so it's is just to avoid another 'create()'. Would you prefer that?
>
> Parameters could be removed - @Bartek Plotka, mind removing that one? :)
sure (:
> On May 28, 2015, 12:24 a.m., Vinod Kone wrote:
> > src/tests/resource_estimator.hpp, lines 55-57
> > <https://reviews.apache.org/r/34662/diff/6/?file=973217#file973217line55>
> >
> > Hmm. I'm confused. Why the change in semantics here?
>
> Niklas Nielsen wrote:
> The typed tests for modules will give you a ResourceEstimator - not a TestResourceEstimator. Therefore, we cannot have the 'estimate()' convenience method on the estimator. Does this make sense?
Exactly!
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 28, 2015, 12:04 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 12:04 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
> On May 27, 2015, 5:24 p.m., Vinod Kone wrote:
> > src/examples/test_resource_estimator_module.cpp, lines 40-44
> > <https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40>
> >
> > I'm confused. 'parameters' are unused and the passed 'flags' are empty?
>
> Niklas Nielsen wrote:
> TestResourceEstimator need to have a 'create()' which takes a Flags object, to be consistent with modules typed tests; so it's is just to avoid another 'create()'. Would you prefer that?
>
> Parameters could be removed - @Bartek Plotka, mind removing that one? :)
>
> Bartek Plotka wrote:
> sure (:
>
> Bartek Plotka wrote:
> However we would need to create new interface method for Module if we don't want use parameters in that case...
>
> Bartek Plotka wrote:
> @vinod and @niklas please see test_allocator_module.cpp it's quite common because of the convention. Even that parameters are not used.
Sorry - forgot that piece. Yes - you need to take that parameter taken the Module API. Scratch my comment.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
On May 27, 2015, 6:20 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 6:20 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85473
-----------------------------------------------------------
src/examples/test_resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment137051>
I'm confused. 'parameters' are unused and the passed 'flags' are empty?
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137054>
How come we didn't have this problem with TestAllocator?
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137053>
s/consistent/to be consistent/
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137052>
Hmm. I'm confused. Why the change in semantics here?
- Vinod Kone
On May 28, 2015, 12:04 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 12:04 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85533
-----------------------------------------------------------
Patch looks great!
Reviews applied: [34662]
All tests passed.
- Mesos ReviewBot
On May 28, 2015, 1:20 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 1:20 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85613
-----------------------------------------------------------
src/examples/test_resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment137255>
Kill this
src/examples/test_resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment137257>
s/flags//
Fits on one line
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment137258>
s/flags//
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137252>
Kill this
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137253>
remove flags
- Niklas Nielsen
On May 27, 2015, 6:20 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 6:20 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
> On June 1, 2015, 1:55 p.m., Jie Yu wrote:
> > Could you please split out the module definition for ResourceEstimator and make this patch not dependent on r34816? I need this patch to build Fixed Resource Estimator.
Can't we just get this small chain in a good shape and land it? :)
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86068
-----------------------------------------------------------
On May 31, 2015, 11:13 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 11:13 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
> Added *TestNoopResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Jie Yu <yu...@gmail.com>.
> On June 1, 2015, 8:55 p.m., Jie Yu wrote:
> > src/tests/module.hpp, lines 76-84
> > <https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76>
> >
> > Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this.
> >
> > It would be more intuitive if we used template specification.
> >
> > ```
> > template <ResourceEstimator, TestNoopResourceEstimator>
> > class Module
> > {
> > static Try<ResourceEstimator*> create(const Option<std::string>& type)
> > {
> > ...
> > }
> > };
> > ```
>
> Niklas Nielsen wrote:
> When we don't type param the test, we won't need this overload (as far as I can grasp).
True. But "Try<T*> create(logging::Flags flags)" has the same issue. I cannot not tell what this function is used for (i.e., which module).
We don't have to solve it in this review. Just some thoughts on readability:)
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86068
-----------------------------------------------------------
On June 1, 2015, 6:13 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated June 1, 2015, 6:13 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
> Added *TestNoopResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
> On June 1, 2015, 1:55 p.m., Jie Yu wrote:
> > src/tests/module.hpp, lines 76-84
> > <https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76>
> >
> > Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this.
> >
> > It would be more intuitive if we used template specification.
> >
> > ```
> > template <ResourceEstimator, TestNoopResourceEstimator>
> > class Module
> > {
> > static Try<ResourceEstimator*> create(const Option<std::string>& type)
> > {
> > ...
> > }
> > };
> > ```
When we don't type param the test, we won't need this overload (as far as I can grasp).
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86068
-----------------------------------------------------------
On May 31, 2015, 11:13 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 11:13 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
> Added *TestNoopResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86068
-----------------------------------------------------------
Could you please split out the module definition for ResourceEstimator and make this patch not dependent on r34816? I need this patch to build Fixed Resource Estimator.
src/tests/module.hpp
<https://reviews.apache.org/r/34662/#comment137966>
Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this.
It would be more intuitive if we used template specification.
```
template <ResourceEstimator, TestNoopResourceEstimator>
class Module
{
static Try<ResourceEstimator*> create(const Option<std::string>& type)
{
...
}
};
```
src/tests/module.hpp
<https://reviews.apache.org/r/34662/#comment137967>
const Option<std::string>& type please
src/tests/module.hpp
<https://reviews.apache.org/r/34662/#comment137968>
const logging::Flags& flags please
- Jie Yu
On June 1, 2015, 6:13 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated June 1, 2015, 6:13 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
> Added *TestNoopResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86263
-----------------------------------------------------------
Ship it!
Modulo Jie's comments
- Niklas Nielsen
On June 1, 2015, 5:40 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated June 1, 2015, 5:40 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On June 2, 2015, 5:07 p.m., Jie Yu wrote:
> > src/tests/module.hpp, lines 76-79
> > <https://reviews.apache.org/r/34662/diff/14/?file=976049#file976049line76>
> >
> > Do we need this?
Nope, we deleted RE moduled test so in fact we don't - sorry.
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86247
-----------------------------------------------------------
On June 2, 2015, 12:40 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 12:40 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review86247
-----------------------------------------------------------
Ship it!
src/examples/test_resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment138171>
Try<ResourceEstimator*> result = NoopResourceEstimator::create(None());
src/tests/module.hpp
<https://reviews.apache.org/r/34662/#comment138172>
Do we need this?
- Jie Yu
On June 2, 2015, 12:40 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 12:40 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 2, 2015, 8:58 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Jie Yu issues addressed.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 2, 2015, 12:40 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Removed unused includes.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 2, 2015, 12:28 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 2, 2015, 12:19 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Issue addressed.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description (updated)
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 1, 2015, 6:13 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description (updated)
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Added *TestNoopResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
Diffs
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated June 1, 2015, 6:12 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Updated after the 34816 changes.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Added *TestResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 29, 2015, 7:10 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
This patch now depends on the ResourceEstimator tests. Using mocked class and typed test to test RE module.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description (updated)
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module.
Added *TestResourceEstimator* module to typed_tests in *oversubscription_tests.cpp*
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 28, 2015, 9:27 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Some slight refactoring.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 28, 2015, 9:23 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Remove unused flag. TODO: Mock ResourceEstimator to inject resources.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85606
-----------------------------------------------------------
Ship it!
Ship It!
- Niklas Nielsen
On May 27, 2015, 6:20 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 6:20 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 28, 2015, 1:20 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Changed comment.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 28, 2015, 12:04 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Refactored RETypes
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 28669f6d2d1a6a9f616ff4a288b9038235a23595
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85413
-----------------------------------------------------------
Do you want to wire up module loading in src/slave/resource_estimator.cpp in this patch, or in a follow up? :)
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment137018>
When you resolved the conflict by wrapping the test resource estimator in the new namespace, you shouldn't need 'mesos::internal::tests::ModuleID::'
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment137019>
Let's align like in master_allocator_tests.cpp
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment136975>
Reinsert newline please :)
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137020>
Max line length for comments is 70 cols :)
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137026>
Does it make sense to use the 'slave' namespace instead of introducing a new one?
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137021>
End with period :)
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment137022>
Sorry Bartek, I know I asked for the string version of the factory. I thought we could get rid of the flags based one. Let's kill this and use the slave flags in test_resource_estimator_module.cpp
Can you kill this, if you have the 'factory' below?
- Niklas Nielsen
On May 27, 2015, 9:34 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 9:34 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85420
-----------------------------------------------------------
Patch looks great!
Reviews applied: [34662]
All tests passed.
- Mesos ReviewBot
On May 27, 2015, 4:34 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 4:34 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 27, 2015, 4:34 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Fixed Makefile.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartłomiej Płotka <bw...@gmail.com>.
Thx, found the issue (:
2015-05-27 8:39 GMT-07:00 Niklas Nielsen <ni...@mesosphere.io>:
> +1 to Vinod's suggestion. The buildbot does a dist build, so also make
> sure new public headers are in src/Makefile.am
>
> On 27 May 2015 at 08:32, Vinod Kone <vi...@twitter.com.invalid> wrote:
>
>> You likely forgot to add the file to your patch.
>>
>> @vinodkone
>>
>> > On May 27, 2015, at 2:08 AM, Bartek Plotka <bw...@gmail.com> wrote:
>> >
>> >
>> >
>> >>> On May 27, 2015, 9 a.m., Mesos ReviewBot wrote:
>> >>> Bad patch!
>> >>>
>> >>> Reviews applied: [34662]
>> >>>
>> >>> Failed command: make -j3 distcheck
>> >>>
>> >>> Error:
>> >>> make dist-gzip am__post_remove_distdir='@:'
>> >>> make[1]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
>> >>> if test -d "mesos-0.23.0"; then find "mesos-0.23.0" -type d ! -perm
>> -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.23.0" || { sleep 5 && rm -rf
>> "mesos-0.23.0"; }; else :; fi
>> >>> test -d "mesos-0.23.0" || mkdir "mesos-0.23.0"
>> >>> (cd 3rdparty && make top_distdir=../mesos-0.23.0
>> distdir=../mesos-0.23.0/3rdparty \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[2]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
>> >>> (cd libprocess && make top_distdir=../../mesos-0.23.0
>> distdir=../../mesos-0.23.0/3rdparty/libprocess \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[3]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
>> >>> :
>> >>> test -d "../../mesos-0.23.0/3rdparty/libprocess" || mkdir
>> "../../mesos-0.23.0/3rdparty/libprocess"
>> >>> (cd 3rdparty && make top_distdir=../../../mesos-0.23.0
>> distdir=../../../mesos-0.23.0/3rdparty/libprocess/3rdparty \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[4]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
>> >>> (cd stout && make top_distdir=../../../../mesos-0.23.0
>> distdir=../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[5]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
>> >>> :
>> >>> test -d "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
>> || mkdir "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
>> >>> (cd include && make top_distdir=../../../../../mesos-0.23.0
>> distdir=../../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout/include
>> \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[6]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
>> >>> make[6]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
>> >>> test -n ":" \
>> >>> || find
>> "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" -type d !
>> -perm -755 \
>> >>> -exec chmod u+rwx,go+rx {} \; -o \
>> >>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
>> >>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
>> >>> ! -type d ! -perm -444 -exec /bin/bash
>> /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
>> -c -m a+r {} {} \; \
>> >>> || chmod -R a+r
>> "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
>> >>> make[5]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
>> >>> make[4]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
>> >>> (cd include && make top_distdir=../../../mesos-0.23.0
>> distdir=../../../mesos-0.23.0/3rdparty/libprocess/include \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[4]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
>> >>> make[4]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
>> >>> test -n ":" \
>> >>> || find "../../mesos-0.23.0/3rdparty/libprocess" -type d ! -perm
>> -755 \
>> >>> -exec chmod u+rwx,go+rx {} \; -o \
>> >>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
>> >>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
>> >>> ! -type d ! -perm -444 -exec /bin/bash
>> /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
>> -c -m a+r {} {} \; \
>> >>> || chmod -R a+r "../../mesos-0.23.0/3rdparty/libprocess"
>> >>> make[3]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
>> >>> make[2]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
>> >>> (cd src && make top_distdir=../mesos-0.23.0
>> distdir=../mesos-0.23.0/src \
>> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
>> distdir)
>> >>> make[2]: Entering directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
>> >>> make[2]: *** No rule to make target
>> `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop.
>> >>> make[2]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
>> >>> make[1]: *** [distdir] Error 1
>> >>> make[1]: Leaving directory
>> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
>> >>> make: *** [dist] Error 2
>> >
>> > Strange - i'm sure it works on my machine... any idea?
>> >
>> > "No rule to make target `examples/test_resource_estimator_module.hpp',
>> needed by `distdir'. Stop"
>> >
>> >
>> > - Bartek
>> >
>> >
>> > -----------------------------------------------------------
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/34662/#review85336
>> > -----------------------------------------------------------
>> >
>> >
>> >> On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
>> >>
>> >> -----------------------------------------------------------
>> >> This is an automatically generated e-mail. To reply, visit:
>> >> https://reviews.apache.org/r/34662/
>> >> -----------------------------------------------------------
>> >>
>> >> (Updated May 27, 2015, 1:34 a.m.)
>> >>
>> >>
>> >> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and
>> Vinod Kone.
>> >>
>> >>
>> >> Bugs: MESOS-2650
>> >> https://issues.apache.org/jira/browse/MESOS-2650
>> >>
>> >>
>> >> Repository: mesos
>> >>
>> >>
>> >> Description
>> >> -------
>> >>
>> >> Added *ResourceEstimator* (RE) module interface.
>> >> Added *TestResourceEstimator* example module. (Test
>> *ResourceEstimator* module)
>> >> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
>> >> Changed *oversubscription_tests* to be typed_tested (for normal RE and
>> RE module)
>> >>
>> >> NOTE: The example modules were good enough for other modules' unit
>> tests, however RE had to be extended - to push particular *Resources* to
>> slave.
>> >>
>> >> In future if we implement
>> https://issues.apache.org/jira/browse/MESOS-2764, we will be able to
>> inject stubed *Resources* in better way.
>> >>
>> >>
>> >> Diffs
>> >> -----
>> >>
>> >> include/mesos/module/resource_estimator.hpp PRE-CREATION
>> >> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
>> >> src/examples/test_resource_estimator_module.cpp PRE-CREATION
>> >> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
>> >> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
>> >> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
>> >> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
>> >> src/tests/oversubscription_tests.cpp
>> 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
>> >> src/tests/resource_estimator.hpp PRE-CREATION
>> >>
>> >> Diff: https://reviews.apache.org/r/34662/diff/
>> >>
>> >>
>> >> Testing
>> >> -------
>> >>
>> >> make check
>> >>
>> >>
>> >> Thanks,
>> >>
>> >> Bartek Plotka
>> >
>>
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@mesosphere.io>.
+1 to Vinod's suggestion. The buildbot does a dist build, so also make sure
new public headers are in src/Makefile.am
On 27 May 2015 at 08:32, Vinod Kone <vi...@twitter.com.invalid> wrote:
> You likely forgot to add the file to your patch.
>
> @vinodkone
>
> > On May 27, 2015, at 2:08 AM, Bartek Plotka <bw...@gmail.com> wrote:
> >
> >
> >
> >>> On May 27, 2015, 9 a.m., Mesos ReviewBot wrote:
> >>> Bad patch!
> >>>
> >>> Reviews applied: [34662]
> >>>
> >>> Failed command: make -j3 distcheck
> >>>
> >>> Error:
> >>> make dist-gzip am__post_remove_distdir='@:'
> >>> make[1]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
> >>> if test -d "mesos-0.23.0"; then find "mesos-0.23.0" -type d ! -perm
> -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.23.0" || { sleep 5 && rm -rf
> "mesos-0.23.0"; }; else :; fi
> >>> test -d "mesos-0.23.0" || mkdir "mesos-0.23.0"
> >>> (cd 3rdparty && make top_distdir=../mesos-0.23.0
> distdir=../mesos-0.23.0/3rdparty \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[2]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
> >>> (cd libprocess && make top_distdir=../../mesos-0.23.0
> distdir=../../mesos-0.23.0/3rdparty/libprocess \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[3]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
> >>> :
> >>> test -d "../../mesos-0.23.0/3rdparty/libprocess" || mkdir
> "../../mesos-0.23.0/3rdparty/libprocess"
> >>> (cd 3rdparty && make top_distdir=../../../mesos-0.23.0
> distdir=../../../mesos-0.23.0/3rdparty/libprocess/3rdparty \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[4]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
> >>> (cd stout && make top_distdir=../../../../mesos-0.23.0
> distdir=../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[5]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
> >>> :
> >>> test -d "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
> || mkdir "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
> >>> (cd include && make top_distdir=../../../../../mesos-0.23.0
> distdir=../../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout/include
> \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[6]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
> >>> make[6]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
> >>> test -n ":" \
> >>> || find
> "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" -type d !
> -perm -755 \
> >>> -exec chmod u+rwx,go+rx {} \; -o \
> >>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
> >>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
> >>> ! -type d ! -perm -444 -exec /bin/bash
> /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
> -c -m a+r {} {} \; \
> >>> || chmod -R a+r
> "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
> >>> make[5]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
> >>> make[4]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
> >>> (cd include && make top_distdir=../../../mesos-0.23.0
> distdir=../../../mesos-0.23.0/3rdparty/libprocess/include \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[4]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
> >>> make[4]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
> >>> test -n ":" \
> >>> || find "../../mesos-0.23.0/3rdparty/libprocess" -type d ! -perm
> -755 \
> >>> -exec chmod u+rwx,go+rx {} \; -o \
> >>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
> >>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
> >>> ! -type d ! -perm -444 -exec /bin/bash
> /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
> -c -m a+r {} {} \; \
> >>> || chmod -R a+r "../../mesos-0.23.0/3rdparty/libprocess"
> >>> make[3]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
> >>> make[2]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
> >>> (cd src && make top_distdir=../mesos-0.23.0
> distdir=../mesos-0.23.0/src \
> >>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=:
> distdir)
> >>> make[2]: Entering directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
> >>> make[2]: *** No rule to make target
> `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop.
> >>> make[2]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
> >>> make[1]: *** [distdir] Error 1
> >>> make[1]: Leaving directory
> `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
> >>> make: *** [dist] Error 2
> >
> > Strange - i'm sure it works on my machine... any idea?
> >
> > "No rule to make target `examples/test_resource_estimator_module.hpp',
> needed by `distdir'. Stop"
> >
> >
> > - Bartek
> >
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/34662/#review85336
> > -----------------------------------------------------------
> >
> >
> >> On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/34662/
> >> -----------------------------------------------------------
> >>
> >> (Updated May 27, 2015, 1:34 a.m.)
> >>
> >>
> >> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and
> Vinod Kone.
> >>
> >>
> >> Bugs: MESOS-2650
> >> https://issues.apache.org/jira/browse/MESOS-2650
> >>
> >>
> >> Repository: mesos
> >>
> >>
> >> Description
> >> -------
> >>
> >> Added *ResourceEstimator* (RE) module interface.
> >> Added *TestResourceEstimator* example module. (Test *ResourceEstimator*
> module)
> >> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> >> Changed *oversubscription_tests* to be typed_tested (for normal RE and
> RE module)
> >>
> >> NOTE: The example modules were good enough for other modules' unit
> tests, however RE had to be extended - to push particular *Resources* to
> slave.
> >>
> >> In future if we implement
> https://issues.apache.org/jira/browse/MESOS-2764, we will be able to
> inject stubed *Resources* in better way.
> >>
> >>
> >> Diffs
> >> -----
> >>
> >> include/mesos/module/resource_estimator.hpp PRE-CREATION
> >> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> >> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> >> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> >> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> >> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> >> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> >> src/tests/oversubscription_tests.cpp
> 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> >> src/tests/resource_estimator.hpp PRE-CREATION
> >>
> >> Diff: https://reviews.apache.org/r/34662/diff/
> >>
> >>
> >> Testing
> >> -------
> >>
> >> make check
> >>
> >>
> >> Thanks,
> >>
> >> Bartek Plotka
> >
>
Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module
Posted by Vinod Kone <vi...@twitter.com.INVALID>.
You likely forgot to add the file to your patch.
@vinodkone
> On May 27, 2015, at 2:08 AM, Bartek Plotka <bw...@gmail.com> wrote:
>
>
>
>>> On May 27, 2015, 9 a.m., Mesos ReviewBot wrote:
>>> Bad patch!
>>>
>>> Reviews applied: [34662]
>>>
>>> Failed command: make -j3 distcheck
>>>
>>> Error:
>>> make dist-gzip am__post_remove_distdir='@:'
>>> make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
>>> if test -d "mesos-0.23.0"; then find "mesos-0.23.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.23.0" || { sleep 5 && rm -rf "mesos-0.23.0"; }; else :; fi
>>> test -d "mesos-0.23.0" || mkdir "mesos-0.23.0"
>>> (cd 3rdparty && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/3rdparty \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
>>> (cd libprocess && make top_distdir=../../mesos-0.23.0 distdir=../../mesos-0.23.0/3rdparty/libprocess \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
>>> :
>>> test -d "../../mesos-0.23.0/3rdparty/libprocess" || mkdir "../../mesos-0.23.0/3rdparty/libprocess"
>>> (cd 3rdparty && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/3rdparty \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
>>> (cd stout && make top_distdir=../../../../mesos-0.23.0 distdir=../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[5]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
>>> :
>>> test -d "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" || mkdir "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
>>> (cd include && make top_distdir=../../../../../mesos-0.23.0 distdir=../../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout/include \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[6]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
>>> make[6]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
>>> test -n ":" \
>>> || find "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" -type d ! -perm -755 \
>>> -exec chmod u+rwx,go+rx {} \; -o \
>>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
>>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
>>> ! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh -c -m a+r {} {} \; \
>>> || chmod -R a+r "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
>>> make[5]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
>>> make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
>>> (cd include && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/include \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
>>> make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
>>> test -n ":" \
>>> || find "../../mesos-0.23.0/3rdparty/libprocess" -type d ! -perm -755 \
>>> -exec chmod u+rwx,go+rx {} \; -o \
>>> ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
>>> ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
>>> ! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh -c -m a+r {} {} \; \
>>> || chmod -R a+r "../../mesos-0.23.0/3rdparty/libprocess"
>>> make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
>>> make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
>>> (cd src && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/src \
>>> am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
>>> make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
>>> make[2]: *** No rule to make target `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop.
>>> make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
>>> make[1]: *** [distdir] Error 1
>>> make[1]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
>>> make: *** [dist] Error 2
>
> Strange - i'm sure it works on my machine... any idea?
>
> "No rule to make target `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop"
>
>
> - Bartek
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/#review85336
> -----------------------------------------------------------
>
>
>> On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/34662/
>> -----------------------------------------------------------
>>
>> (Updated May 27, 2015, 1:34 a.m.)
>>
>>
>> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>>
>>
>> Bugs: MESOS-2650
>> https://issues.apache.org/jira/browse/MESOS-2650
>>
>>
>> Repository: mesos
>>
>>
>> Description
>> -------
>>
>> Added *ResourceEstimator* (RE) module interface.
>> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
>> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
>> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>>
>> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>>
>> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>>
>>
>> Diffs
>> -----
>>
>> include/mesos/module/resource_estimator.hpp PRE-CREATION
>> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
>> src/examples/test_resource_estimator_module.cpp PRE-CREATION
>> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
>> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
>> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
>> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
>> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
>> src/tests/resource_estimator.hpp PRE-CREATION
>>
>> Diff: https://reviews.apache.org/r/34662/diff/
>>
>>
>> Testing
>> -------
>>
>> make check
>>
>>
>> Thanks,
>>
>> Bartek Plotka
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 27, 2015, 9 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> >
> > Reviews applied: [34662]
> >
> > Failed command: make -j3 distcheck
> >
> > Error:
> > make dist-gzip am__post_remove_distdir='@:'
> > make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
> > if test -d "mesos-0.23.0"; then find "mesos-0.23.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.23.0" || { sleep 5 && rm -rf "mesos-0.23.0"; }; else :; fi
> > test -d "mesos-0.23.0" || mkdir "mesos-0.23.0"
> > (cd 3rdparty && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/3rdparty \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
> > (cd libprocess && make top_distdir=../../mesos-0.23.0 distdir=../../mesos-0.23.0/3rdparty/libprocess \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
> > :
> > test -d "../../mesos-0.23.0/3rdparty/libprocess" || mkdir "../../mesos-0.23.0/3rdparty/libprocess"
> > (cd 3rdparty && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/3rdparty \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
> > (cd stout && make top_distdir=../../../../mesos-0.23.0 distdir=../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[5]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
> > :
> > test -d "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" || mkdir "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
> > (cd include && make top_distdir=../../../../../mesos-0.23.0 distdir=../../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout/include \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[6]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
> > make[6]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
> > test -n ":" \
> > || find "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" -type d ! -perm -755 \
> > -exec chmod u+rwx,go+rx {} \; -o \
> > ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
> > ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
> > ! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh -c -m a+r {} {} \; \
> > || chmod -R a+r "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
> > make[5]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
> > make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
> > (cd include && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/include \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
> > make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
> > test -n ":" \
> > || find "../../mesos-0.23.0/3rdparty/libprocess" -type d ! -perm -755 \
> > -exec chmod u+rwx,go+rx {} \; -o \
> > ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
> > ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
> > ! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh -c -m a+r {} {} \; \
> > || chmod -R a+r "../../mesos-0.23.0/3rdparty/libprocess"
> > make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
> > make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
> > (cd src && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/src \
> > am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
> > make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
> > make[2]: *** No rule to make target `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop.
> > make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
> > make[1]: *** [distdir] Error 1
> > make[1]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
> > make: *** [dist] Error 2
Strange - i'm sure it works on my machine... any idea?
"No rule to make target `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop"
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85336
-----------------------------------------------------------
On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 1:34 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85336
-----------------------------------------------------------
Bad patch!
Reviews applied: [34662]
Failed command: make -j3 distcheck
Error:
make dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.23.0"; then find "mesos-0.23.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.23.0" || { sleep 5 && rm -rf "mesos-0.23.0"; }; else :; fi
test -d "mesos-0.23.0" || mkdir "mesos-0.23.0"
(cd 3rdparty && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/3rdparty \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
(cd libprocess && make top_distdir=../../mesos-0.23.0 distdir=../../mesos-0.23.0/3rdparty/libprocess \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d "../../mesos-0.23.0/3rdparty/libprocess" || mkdir "../../mesos-0.23.0/3rdparty/libprocess"
(cd 3rdparty && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/3rdparty \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
(cd stout && make top_distdir=../../../../mesos-0.23.0 distdir=../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
:
test -d "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" || mkdir "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
(cd include && make top_distdir=../../../../../mesos-0.23.0 distdir=../../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout/include \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[6]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
test -n ":" \
|| find "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
! -type d ! -perm -400 -exec chmod a+r {} \; -o \
! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh -c -m a+r {} {} \; \
|| chmod -R a+r "../../../../mesos-0.23.0/3rdparty/libprocess/3rdparty/stout"
make[5]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
(cd include && make top_distdir=../../../mesos-0.23.0 distdir=../../../mesos-0.23.0/3rdparty/libprocess/include \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
test -n ":" \
|| find "../../mesos-0.23.0/3rdparty/libprocess" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
! -type d ! -perm -400 -exec chmod a+r {} \; -o \
! -type d ! -perm -444 -exec /bin/bash /home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh -c -m a+r {} {} \; \
|| chmod -R a+r "../../mesos-0.23.0/3rdparty/libprocess"
make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
(cd src && make top_distdir=../mesos-0.23.0 distdir=../mesos-0.23.0/src \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[2]: *** No rule to make target `examples/test_resource_estimator_module.hpp', needed by `distdir'. Stop.
make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
make: *** [dist] Error 2
- Mesos ReviewBot
On May 27, 2015, 1:34 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 1:34 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 27, 2015, 1:34 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
All nnielsen issues addressed. Additionally made includes to be in alphabetical order.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 27, 2015, 12:15 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description (updated)
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 27, 2015, 12:06 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Now example/test_resource_estimator_module.cpp is responsible for unit tests.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 26, 2015, 8:13 p.m., Niklas Nielsen wrote:
> > src/tests/resource_estimator.hpp, line 40
> > <https://reviews.apache.org/r/34662/diff/2/?file=971547#file971547line40>
> >
> > Put { on a newline
> > Why have create take flags? We tried to make it only take a string.
Yeah, but it was needed since the tests::Module::create receives Flag as parameter, so that test class needs to be consistent to have working typed test on both RE module and normal RE.
However, in current review req there are both static "create" methods: one with string, second with Flag. Is that resolves your issue?
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85225
-----------------------------------------------------------
On May 27, 2015, 12:15 a.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 12:15 a.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Test *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/#review85225
-----------------------------------------------------------
src/Makefile.am
<https://reviews.apache.org/r/34662/#comment136757>
Why have both?
src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/34662/#comment136758>
Newline before
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136756>
kill newline
src/tests/resource_estimator.hpp
<https://reviews.apache.org/r/34662/#comment136739>
Put { on a newline
Why have create take flags? We tried to make it only take a string.
src/tests/resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment136742>
2 space indent
src/tests/resource_estimator_module.cpp
<https://reviews.apache.org/r/34662/#comment136741>
Skip the ' - ' or kill the 'for unit tests' to be consistent with the other test modules
- Niklas Nielsen
On May 25, 2015, 10:29 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> -----------------------------------------------------------
>
> (Updated May 25, 2015, 10:29 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
>
>
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added *ResourceEstimator* (RE) module interface.
> Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
> Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
> Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
> Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
>
> NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
>
> In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
>
>
> Diffs
> -----
>
> include/mesos/module/resource_estimator.hpp PRE-CREATION
> src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
> src/examples/test_resource_estimator_module.cpp PRE-CREATION
> src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
> src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
> src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
> src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
> src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
> src/tests/resource_estimator.hpp PRE-CREATION
> src/tests/resource_estimator_module.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34662/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34662: Modularized ResourceEstimator and added
test for RE module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 26, 2015, 5:29 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Summary (updated)
-----------------
Modularized ResourceEstimator and added test for RE module
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
src/tests/resource_estimator_module.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34662: Modularized ResourceEstimator and added
test for that module
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34662/
-----------------------------------------------------------
(Updated May 26, 2015, 4:36 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone.
Changes
-------
Add some comments.
Bugs: MESOS-2650
https://issues.apache.org/jira/browse/MESOS-2650
Repository: mesos
Description
-------
Added *ResourceEstimator* (RE) module interface.
Added *TestResourceEstimator* example module. (Noop *ResourceEstimator* module)
Moved *TestResourceEstimator* to seperate file from *tests/mesos.hpp*
Added *DummyTestResourceEstimator* module for unit tests purpose. (Module with *TestResourceEstimator* logic - that's why it has to be in seperate file)
Changed *oversubscription_tests* to be typed_tested (for normal RE and RE module)
NOTE: The example modules were good enough for other modules' unit tests, however RE had to be extended - to push particular *Resources* to slave. That's why it was necessary to add new "Dummy" *TestResourceEstimator* module.
In future if we implement https://issues.apache.org/jira/browse/MESOS-2764, we will be able to inject stubed *Resources* in better way.
Diffs (updated)
-----
include/mesos/module/resource_estimator.hpp PRE-CREATION
src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19
src/examples/test_resource_estimator_module.cpp PRE-CREATION
src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be
src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba
src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8
src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29
src/tests/oversubscription_tests.cpp 75c25b04c1e6a8e0e7e8fd55440743fe1699af88
src/tests/resource_estimator.hpp PRE-CREATION
src/tests/resource_estimator_module.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/34662/diff/
Testing
-------
make check
Thanks,
Bartek Plotka