Re: Review Request 34317: Updated callers of os::getenv() in /src and removed calls to os::hasenv()

2015-05-25 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34317/
---

(Updated May 25, 2015, 4:44 p.m.)


Review request for mesos.


Summary (updated)
-

Updated callers of os::getenv() in /src and removed calls to os::hasenv()


Bugs: MESOS-994
https://issues.apache.org/jira/browse/MESOS-994


Repository: mesos


Description
---

Updated callers of os::getenv() in /src, remove os::hasenv() calls.


Diffs
-

  src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
  src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
  src/examples/docker_no_executor_framework.cpp 
df8b9109e15f6566f3f33428e8cf0014c520ea7b 
  src/examples/load_generator_framework.cpp 
be1a3bf5f16bd811cb4039c8f15478183712a426 
  src/examples/long_lived_framework.cpp 
d1d577e04be24b781ad279a06fc07611e2a0122b 
  src/examples/low_level_scheduler_libprocess.cpp 
bee2e7ef8432cc42733260c668f1100c68f73b8d 
  src/examples/low_level_scheduler_pthread.cpp 
fb8cd66c2e94270971184c1e3dcc92eccab8b223 
  src/examples/no_executor_framework.cpp 
37001c389f31f9f1dafe6d7f3eb17adc2e369057 
  src/examples/test_framework.cpp 9f4b53e44e40709c01e34cdaa9d0a9ac57b7a768 
  src/exec/exec.cpp a22e8bbf146983937e6fae00af601f9e886e88f1 
  src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
  src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
  src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/slave/containerizer/containerizer.cpp 
4d66e767de1f877cb66b37826ba7c9d00639a7c0 
  src/slave/containerizer/docker.cpp bd58f94c53ab25c1ae9b8d9eeae5a887faf1fe80 
  src/tests/anonymous_tests.cpp 12d4eb438c3d5f539ebf22bb159a98ef9141e224 
  src/tests/environment.cpp e002363f8fbc63936211ff4c2dd36bfc5b452d17 

Diff: https://reviews.apache.org/r/34317/diff/


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 34319: Refactored os::getenv() to return an option and updated its callers in stout.

2015-05-25 Thread Greg Mann


 On May 20, 2015, 5:50 p.m., Timothy Chen wrote:
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 157
  https://reviews.apache.org/r/34319/diff/3/?file=962168#file962168line157
 
  Btw after looking at your other patches, I realize we don't really need 
  hasenv anymore right? And I see you've been replacing hasenv with getenv, I 
  think why not just remove hasenv all together?
 
 Greg Mann wrote:
 Good call! I think that os::getenv().isSome() eliminates the need for 
 os::hasenv. I can remove it and update the patches.

Done.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34319/#review84553
---


On May 17, 2015, 1:40 p.m., Greg Mann wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34319/
 ---
 
 (Updated May 17, 2015, 1:40 p.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-994
 https://issues.apache.org/jira/browse/MESOS-994
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Refactored os::getenv to return an option and updated os::path accordingly.
 
 
 Diffs
 -
 
   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
 d2ca4be90edb134465751d093b61e84fd6b16f1c 
 
 Diff: https://reviews.apache.org/r/34319/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Greg Mann
 




Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-25 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34317/
---

(Updated May 25, 2015, 4:42 p.m.)


Review request for mesos.


Bugs: MESOS-994
https://issues.apache.org/jira/browse/MESOS-994


Repository: mesos


Description (updated)
---

Updated callers of os::getenv() in /src, remove os::hasenv() calls.


Diffs
-

  src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
  src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
  src/examples/docker_no_executor_framework.cpp 
df8b9109e15f6566f3f33428e8cf0014c520ea7b 
  src/examples/load_generator_framework.cpp 
be1a3bf5f16bd811cb4039c8f15478183712a426 
  src/examples/long_lived_framework.cpp 
d1d577e04be24b781ad279a06fc07611e2a0122b 
  src/examples/low_level_scheduler_libprocess.cpp 
bee2e7ef8432cc42733260c668f1100c68f73b8d 
  src/examples/low_level_scheduler_pthread.cpp 
fb8cd66c2e94270971184c1e3dcc92eccab8b223 
  src/examples/no_executor_framework.cpp 
37001c389f31f9f1dafe6d7f3eb17adc2e369057 
  src/examples/test_framework.cpp 9f4b53e44e40709c01e34cdaa9d0a9ac57b7a768 
  src/exec/exec.cpp a22e8bbf146983937e6fae00af601f9e886e88f1 
  src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
  src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
  src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/slave/containerizer/containerizer.cpp 
4d66e767de1f877cb66b37826ba7c9d00639a7c0 
  src/slave/containerizer/docker.cpp bd58f94c53ab25c1ae9b8d9eeae5a887faf1fe80 
  src/tests/anonymous_tests.cpp 12d4eb438c3d5f539ebf22bb159a98ef9141e224 
  src/tests/environment.cpp e002363f8fbc63936211ff4c2dd36bfc5b452d17 

Diff: https://reviews.apache.org/r/34317/diff/


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 34319: Refactored os::getenv() to return an option and removed os::hasenv()

2015-05-25 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34319/
---

(Updated May 25, 2015, 4:43 p.m.)


Review request for mesos.


Summary (updated)
-

Refactored os::getenv() to return an option and removed os::hasenv()


Bugs: MESOS-994
https://issues.apache.org/jira/browse/MESOS-994


Repository: mesos


Description (updated)
---

Refactored os::getenv to return an option and updated os::path accordingly, and 
removed os::hasenv


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
d2ca4be90edb134465751d093b61e84fd6b16f1c 

Diff: https://reviews.apache.org/r/34319/diff/


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 34318: Update callers of os::getenv() in libprocess.

2015-05-25 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34318/
---

(Updated May 25, 2015, 4:36 p.m.)


Review request for mesos.


Changes
---

Declare and assign Optionstring value on same line.


Bugs: MESOS-994
https://issues.apache.org/jira/browse/MESOS-994


Repository: mesos


Description
---

Update callers of os::getenv() in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp e3de3cd6b536aaaf59784360aed546512dd04dc9 
  3rdparty/libprocess/src/profiler.cpp fe00aa56f587b247659ac9a722179b3809a8fe25 

Diff: https://reviews.apache.org/r/34318/diff/


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-25 Thread Greg Mann


 On May 20, 2015, 5:49 p.m., Timothy Chen wrote:
  src/examples/balloon_framework.cpp, line 207
  https://reviews.apache.org/r/34317/diff/1/?file=962096#file962096line207
 
  Why not Optionstring value = os::getenv(MESOS_BUILD_DIR)?
 
 Greg Mann wrote:
 I think that in the case of a re-used variable, a separate declaration 
 improves readability slightly, but I'm happy to change this if your version 
 is generally preferred.

Moved declaration and assignment onto same line.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34317/#review84551
---


On May 17, 2015, 4:54 a.m., Greg Mann wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34317/
 ---
 
 (Updated May 17, 2015, 4:54 a.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-994
 https://issues.apache.org/jira/browse/MESOS-994
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Updated callers of os::getenv() in /src.
 
 
 Diffs
 -
 
   src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
   src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
   src/examples/docker_no_executor_framework.cpp 
 df8b9109e15f6566f3f33428e8cf0014c520ea7b 
   src/examples/load_generator_framework.cpp 
 be1a3bf5f16bd811cb4039c8f15478183712a426 
   src/examples/long_lived_framework.cpp 
 d1d577e04be24b781ad279a06fc07611e2a0122b 
   src/examples/low_level_scheduler_libprocess.cpp 
 bee2e7ef8432cc42733260c668f1100c68f73b8d 
   src/examples/low_level_scheduler_pthread.cpp 
 fb8cd66c2e94270971184c1e3dcc92eccab8b223 
   src/examples/no_executor_framework.cpp 
 37001c389f31f9f1dafe6d7f3eb17adc2e369057 
   src/examples/test_framework.cpp 9f4b53e44e40709c01e34cdaa9d0a9ac57b7a768 
   src/exec/exec.cpp a22e8bbf146983937e6fae00af601f9e886e88f1 
   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
   src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
   src/tests/anonymous_tests.cpp 12d4eb438c3d5f539ebf22bb159a98ef9141e224 
 
 Diff: https://reviews.apache.org/r/34317/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Greg Mann
 




Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-25 Thread Greg Mann


 On May 20, 2015, 5:49 p.m., Timothy Chen wrote:
  src/examples/low_level_scheduler_libprocess.cpp, line 353
  https://reviews.apache.org/r/34317/diff/1/?file=962100#file962100line353
 
  ditto

Moved declaration and assignment onto same line.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34317/#review84551
---


On May 17, 2015, 4:54 a.m., Greg Mann wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34317/
 ---
 
 (Updated May 17, 2015, 4:54 a.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-994
 https://issues.apache.org/jira/browse/MESOS-994
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Updated callers of os::getenv() in /src.
 
 
 Diffs
 -
 
   src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
   src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
   src/examples/docker_no_executor_framework.cpp 
 df8b9109e15f6566f3f33428e8cf0014c520ea7b 
   src/examples/load_generator_framework.cpp 
 be1a3bf5f16bd811cb4039c8f15478183712a426 
   src/examples/long_lived_framework.cpp 
 d1d577e04be24b781ad279a06fc07611e2a0122b 
   src/examples/low_level_scheduler_libprocess.cpp 
 bee2e7ef8432cc42733260c668f1100c68f73b8d 
   src/examples/low_level_scheduler_pthread.cpp 
 fb8cd66c2e94270971184c1e3dcc92eccab8b223 
   src/examples/no_executor_framework.cpp 
 37001c389f31f9f1dafe6d7f3eb17adc2e369057 
   src/examples/test_framework.cpp 9f4b53e44e40709c01e34cdaa9d0a9ac57b7a768 
   src/exec/exec.cpp a22e8bbf146983937e6fae00af601f9e886e88f1 
   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
   src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
   src/tests/anonymous_tests.cpp 12d4eb438c3d5f539ebf22bb159a98ef9141e224 
 
 Diff: https://reviews.apache.org/r/34317/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Greg Mann
 




Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-25 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34317/
---

(Updated May 25, 2015, 4:41 p.m.)


Review request for mesos.


Changes
---

Moved Optionstring value declarations and assignments onto same line


Bugs: MESOS-994
https://issues.apache.org/jira/browse/MESOS-994


Repository: mesos


Description
---

Updated callers of os::getenv() in /src.


Diffs (updated)
-

  src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
  src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad 
  src/examples/docker_no_executor_framework.cpp 
df8b9109e15f6566f3f33428e8cf0014c520ea7b 
  src/examples/load_generator_framework.cpp 
be1a3bf5f16bd811cb4039c8f15478183712a426 
  src/examples/long_lived_framework.cpp 
d1d577e04be24b781ad279a06fc07611e2a0122b 
  src/examples/low_level_scheduler_libprocess.cpp 
bee2e7ef8432cc42733260c668f1100c68f73b8d 
  src/examples/low_level_scheduler_pthread.cpp 
fb8cd66c2e94270971184c1e3dcc92eccab8b223 
  src/examples/no_executor_framework.cpp 
37001c389f31f9f1dafe6d7f3eb17adc2e369057 
  src/examples/test_framework.cpp 9f4b53e44e40709c01e34cdaa9d0a9ac57b7a768 
  src/exec/exec.cpp a22e8bbf146983937e6fae00af601f9e886e88f1 
  src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
  src/jvm/jvm.cpp d33a655dd46dc52f23905eb53de6530fa7b66a6c 
  src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/slave/containerizer/containerizer.cpp 
4d66e767de1f877cb66b37826ba7c9d00639a7c0 
  src/slave/containerizer/docker.cpp bd58f94c53ab25c1ae9b8d9eeae5a887faf1fe80 
  src/tests/anonymous_tests.cpp 12d4eb438c3d5f539ebf22bb159a98ef9141e224 
  src/tests/environment.cpp e002363f8fbc63936211ff4c2dd36bfc5b452d17 

Diff: https://reviews.apache.org/r/34317/diff/


Testing
---

make check


Thanks,

Greg Mann



Review Request 34655: Use relative url in /help generated links point

2015-05-25 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34655/
---

Review request for mesos and Adam B.


Bugs: MESOS-2748
https://issues.apache.org/jira/browse/MESOS-2748


Repository: mesos


Description
---

Use relative url in /help generated links point


Diffs
-

  3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c 

Diff: https://reviews.apache.org/r/34655/diff/


Testing
---


Thanks,

haosdent huang



Re: Review Request 34645: Update existing lambdas to meet style guide

2015-05-25 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34645/#review85137
---


This is an interesting case. We have a proxy to another function, rather than 
the implementation of that function as a lambda.
I'm curious what the community's view is on using the proxy lambda approach as 
per your patch, versus a `std::bind`. I think the lambda is more readable, the 
bind is more explicit about what is going on :-)
Depending on the way the community votes, I would add a comment here just 
stating that this is a proxy lambda. What do you think?


src/master/master.cpp
https://reviews.apache.org/r/34645/#comment136612

1) I don't think this will compile. We need to capture `this` rather than 
`allocator` if we want to access a class member of the member function.
2) We can use `string` here rather than `std::string` since we are in an 
implementation file that has declared `using std::string`


- Joris Van Remoortere


On May 24, 2015, 4:53 p.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34645/
 ---
 
 (Updated May 24, 2015, 4:53 p.m.)
 
 
 Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
 
 
 Bugs: MESOS-2670
 https://issues.apache.org/jira/browse/MESOS-2670
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Update existing lambdas to meet style guide
 
 
 Diffs
 -
 
   src/master/master.cpp 1526f59e7c6b135657550eab2ca46216923a01f6 
 
 Diff: https://reviews.apache.org/r/34645/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 haosdent huang
 




Review Request 34662: Modularized ResourceEstimator and added test for that module

2015-05-25 Thread Bartek Plotka

---
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 RE module

2015-05-25 Thread Bartek Plotka

---
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 34631: Added QoS Controller.

2015-05-25 Thread Bartek Plotka

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34631/#review85150
---



include/mesos/slave/qos_controller.hpp
https://reviews.apache.org/r/34631/#comment136645

I think it's better idea to do it ASAP.. We produce more and more code, 
which we will have to modify after we expose that Resource Usage  (same 
situation as with ResourceEstimator) - and we are aware of that. Maybe we 
should do that in the first place?


- Bartek Plotka


On May 23, 2015, 5:06 p.m., Niklas Nielsen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34631/
 ---
 
 (Updated May 23, 2015, 5:06 p.m.)
 
 
 Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added QoS Controller class and NOOP controller implementation
 
 
 Diffs
 -
 
   include/mesos/slave/qos_controller.hpp PRE-CREATION 
   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
   src/slave/qos_controller.hpp PRE-CREATION 
   src/slave/qos_controller.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34631/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Niklas Nielsen
 




Re: Review Request 34662: Modularized ResourceEstimator and added test for that module

2015-05-25 Thread Szymon Konefal

---
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 that module

2015-05-25 Thread Bartek Plotka

---
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