Re: Review Request 35118: Made updateSlave() update its 'totalResources'.
On June 5, 2015, 2:58 p.m., Vinod Kone wrote: src/master/master.cpp, line 3517 https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3517 while you are at it, can you just send slave-totalResources here instead of oversubscribedResources? this will address my TODO on this method. The allocator expects only oversubscribedResources and does a CHECK on it. The logic in Allocator::updateSlave(...) needs a little tweaking as well. I'd be happy to do it in a subsequent review. SG? - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35118/#review86850 --- On June 5, 2015, 2:09 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35118/ --- (Updated June 5, 2015, 2:09 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2776 https://issues.apache.org/jira/browse/MESOS-2776 Repository: mesos Description --- - This way Master::Slave::totalResources includes revocable resources, which we need for metrics for revocable resources. - Changed updateSlave() argument to use `const Resources oversubscribedResources` instead of `const std::vectorResource oversubscribedResources` because `Resources` provides convenience methods such as `revocable()`. Diffs - src/master/master.hpp deeb0d8c87a13315206556e1d0974cdd13e8224f src/master/master.cpp be0db42da3c59761aa154439653d715556465256 Diff: https://reviews.apache.org/r/35118/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 35119: Introduced metrics for revocable resources.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35119/ --- (Updated June 7, 2015, 1:32 p.m.) Review request for mesos and Vinod Kone. Changes --- Vinod's comments. Bugs: MESOS-2776 https://issues.apache.org/jira/browse/MESOS-2776 Repository: mesos Description --- state.json changes are in a subsequent review. Diffs (updated) - src/master/master.hpp deeb0d8c87a13315206556e1d0974cdd13e8224f src/master/master.cpp d436f845341ca33a534127da3bad8d8b2aa1175b src/master/metrics.hpp 833033c1912daee429b45423d24d365d8699a428 src/master/metrics.cpp 264252c5159990fdf7a4569933a305d07bd7dd6e src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35119/diff/ Testing --- make check. - Modified a test to test the `total` resources metrics. - We don't have unit tests that use the revocable resources yet, when we add that we should check `used` resources metrics too. Thanks, Jiang Yan Xu
Re: Review Request 35194: Exposing Resources along with ResourceStatistics
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35194/#review86971 --- Patch looks great! Reviews applied: [35194] All tests passed. - Mesos ReviewBot On June 7, 2015, 4:37 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35194/ --- (Updated June 7, 2015, 4:37 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-2741 https://issues.apache.org/jira/browse/MESOS-2741 Repository: mesos Description --- Exposing Resources along with ResourceStatistics Diffs - include/mesos/mesos.proto 6186c92c6fe9cf8fa136d93d5af43e0377406baf src/slave/containerizer/docker.cpp 00db9811824995fe19a6143aa2bcba3733a93147 src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712 Diff: https://reviews.apache.org/r/35194/diff/ Testing --- make check Thanks, haosdent huang
Review Request 35194: Exposing Resources along with ResourceStatistics
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35194/ --- Review request for mesos and Jie Yu. Bugs: MESOS-2741 https://issues.apache.org/jira/browse/MESOS-2741 Repository: mesos Description --- Exposing Resources along with ResourceStatistics Diffs - include/mesos/mesos.proto 6186c92c6fe9cf8fa136d93d5af43e0377406baf src/slave/containerizer/docker.cpp 00db9811824995fe19a6143aa2bcba3733a93147 src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712 Diff: https://reviews.apache.org/r/35194/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 33339: Add a Java example framework to test persistent volumes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9/#review86956 --- Patch looks great! Reviews applied: [9] All tests passed. - Mesos ReviewBot On June 7, 2015, 8:10 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9/ --- (Updated June 7, 2015, 8:10 a.m.) Review request for mesos, Jie Yu and Vinod Kone. Bugs: MESOS-2610 https://issues.apache.org/jira/browse/MESOS-2610 Repository: mesos Description --- Add a Java example framework to test persistent volumes. Diffs - configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 src/Makefile.am ec7f41f2114d02d3d5a1434d9799f15fe9331917 src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION src/examples/java/test-persistent-volume-framework.in PRE-CREATION src/tests/examples_tests.cpp f85b81562158c5499e9804d8d7b6811bb0a3ef16 src/tests/java_persistent_volume_framework_test.sh PRE-CREATION Diff: https://reviews.apache.org/r/9/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34646: Redirect to the leader master when current master is not a leader.
On June 1, 2015, 8:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have a newbie problem. In the unit test, do I start a multi-masters cluster to verify this? I search the exist unit tests, could not find a exist snippet to start a multi-masters cluster. Adam B wrote: Good question. Our unit test macros are not well documented, so I usually find a couple of example tests to copy from. Check out MasterAllocatorTest.FrameworkReregistersFirst and MasterAllocatorTest.SlaveReregistersFirst, which actually do failover and leader-election between two masters. https://github.com/apache/mesos/blob/0.22.1/src/tests/master_allocator_tests.cpp#L1295 haosdent huang wrote: @adam-mesos Thank you very much! I miss them when I read the unit tests. Let me try to add a unit test. @adam-mesos In `MasterAllocatorTest.FrameworkReregistersFirst` and `MasterAllocatorTest.SlaveReregistersFirst`, before them start the second Master, it need call `this-ShutdownMasters();`. So there is only one master when test. But this test case need two alive masters, I try to start two masters, but it failed. So we have to change some thing make it possible to start two alive masters in unit tests. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/#review85955 --- On June 1, 2015, 3:07 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/ --- (Updated June 1, 2015, 3:07 p.m.) Review request for mesos and Adam B. Bugs: MESOS-1865 https://issues.apache.org/jira/browse/MESOS-1865 Repository: mesos Description --- Return empty task list when current master is not a leader. Diffs - src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 src/master/master.hpp c0cc2933a2cc094401f633df12356bda3d294564 Diff: https://reviews.apache.org/r/34646/diff/ Testing --- make check when current master is not a leader, it would redirect to the leader master. ``` $ curl -i http://master1:5050/master/tasks.json HTTP/1.1 307 Temporary Redirect Date: Mon, 01 Jun 2015 06:30:08 GMT Location: http://master2:5050//master/tasks.json Content-Length: 0 ``` Thanks, haosdent huang
Re: Review Request 34943: Added validation to flags.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34943/#review86981
---
3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
https://reviews.apache.org/r/34943/#comment139192
style question: Why not use static array declaration :
const char *args[] = {
/path/to/program,
blah
};
and also make the function argument const char**?
- Jojy Varghese
On June 5, 2015, 2:48 p.m., Benjamin Hindman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34943/
---
(Updated June 5, 2015, 2:48 p.m.)
Review request for mesos.
Repository: mesos
Description
---
Also refactored existing 'lambda::bind' arguments to use C++11 lambdas,
enabling us to get rid of our loader and stringifier functors.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/Makefile.am
6ac2f04a6a1d8db1725972a3b4b46a0dd734566f
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp
87606d860dea3235ec70d127d3379065d42dc90b
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
ee855da6128c2d671eb2fc7eaa2c0aab2341aebb
3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp
51d3ab020bd2bae1f12b66cac0da5383c813d5d7
3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp
fda5ae1328a23a4371475652f921341dce7448d5
3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
80450185f60c5b273face490e0bb9e695b0cb984
Diff: https://reviews.apache.org/r/34943/diff/
Testing
---
make check
Thanks,
Benjamin Hindman
