Re: Review Request 34894: Add new message for Traffic Control statistics
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34894/#review87062 --- Ship it! Ship It! - Ian Downes On June 3, 2015, 2:54 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34894/ --- (Updated June 3, 2015, 2:54 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Repository: mesos Description --- Add new message for Traffic Control statistics Diffs - include/mesos/mesos.proto 6186c92c6fe9cf8fa136d93d5af43e0377406baf Diff: https://reviews.apache.org/r/34894/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 33339: Add a Java example framework to test persistent volumes.
On June 1, 2015, 11:32 p.m., Marco Massenzio wrote: This is great -sorry it took so long to get to do a review. Thanks for doing it, I'm quite looking forward to using it to learning more about the Persistent Framework :) it would be great if we could have a bit more comments in the code to help all other newbies. My review was fairly narrowly focused on Java style etc. - I'm expecting one of the commiters will be able to give you feedback about using the framework. Jie Yu wrote: Sorry, this is my fault. I really don't have cycle for this at this moment. Marco, mind sherperding this patch? Jie Yu wrote: The logic of this patch should match that in src/examples/persistent_volume_framework.cpp Marco Massenzio wrote: Jie - I would be absolutely happy to do that, with one minor hitch: I'm not a committer :) Happy to help out wherever I can, though! haosdent huang wrote: Thank you very much for your review, let me update it. @marco haosdent huang wrote: @marco Thank you for your review again. But some logic are follow the exists Java examples and persistent_volume_framework.cpp . I am not sure change it are correct or not. Anyway, I think I need to reflect this patch to make it more clear. After I reflect it, I would @you and looking forward your review again. Thanks a lot. I understand that - and therein lies the problem :) As you proved, code in 'examples' is widely used as a template (aka, copied) so it would be great if we could set an... example, by providing great code there. Translating Java almost verbatim from C++ actually results in non-idiomatic constructs that either (a) look weird to Java practitioners or (b) engender bad practices (that are then propagated by the oh, but that's how it's done in the other examples - sounds familiar? :) My suggestions (and I stress it here, they are suggestions - if committers with more Java experience than I are happy with your code, then that's just fine) are to provide some great examples, that improve on existing code. Thanks for doing this! On June 1, 2015, 11:32 p.m., Marco Massenzio wrote: src/examples/java/TestPersistentVolumeFramework.java, line 29 https://reviews.apache.org/r/9/diff/10/?file=941652#file941652line29 as this is an example framework, it'd be great if it could have (extensive) javadocs: newbies and users alike are likely to look inside here first to learn (I sure did :) and providing them with good, extensive guidance as to why stuff is done the way it is, it would be great! Ideally, all public classes + public methods should have javadocs (not sure if this is also in the public google java style guide, it sure was a MUST in our internal one) Thanks! haosdent huang wrote: Acording to the exist java example framework (TestExecutor.java/TestMultipleExecutorsFramework.java/...), don't have javadocs here. Do we still need add javadocs here, or keep same as other exist examples? My preference would be to add documentation and be a good guy to whoever will follow in your footstep and appreciate the comments ;) On June 1, 2015, 11:32 p.m., Marco Massenzio wrote: src/examples/java/TestPersistentVolumeFramework.java, line 110 https://reviews.apache.org/r/9/diff/10/?file=941652#file941652line110 here too haosdent huang wrote: here could not remove because `j` is used in `remains.set(j, newRemain);`, yes, I noticed that - I'm wondering whether there's a more Java-idiomatic way of achieving same. Not a big deal, but definitely something worth considering. On June 1, 2015, 11:32 p.m., Marco Massenzio wrote: src/examples/java/TestPersistentVolumeFramework.java, lines 344-352 https://reviews.apache.org/r/9/diff/10/?file=941652#file941652line344 this is almost identical code to L307-315: how about factoring out to a common `buildTaskInfo(String command)`? haosdent huang wrote: Sure, but it would not match the code ```cpp TaskInfo task; task.set_name(shard.name); task.mutable_task_id()-set_value(UUID::random().toString()); task.mutable_slave_id()-CopyFrom(offer.slave_id()); task.mutable_resources()-CopyFrom(shard.resources); task.mutable_command()-set_value(test -f volume/persisted); ``` in persistent_volume_framework.cpp And, maybe, that's A Good Thing :) On June 1, 2015, 11:32 p.m., Marco Massenzio wrote: src/examples/java/TestPersistentVolumeFramework.java, lines 443-444 https://reviews.apache.org/r/9/diff/10/?file=941652#file941652line443 instead of concatenating string, either use a StringBuilder or, even better: ``` String.format(Received framework message from executor '%s' on slave %s: '%s', executorId, slaveId, data); ``` haosdent huang wrote: Sure, but would not match ``` LOG(INFO) Received
Re: Review Request 35037: Added doxygen link to home.md.
On June 3, 2015, 4:37 p.m., Ben Mahler wrote: docs/home.md, line 36 https://reviews.apache.org/r/35037/diff/1/?file=977892#file977892line36 Should this be linking to the framework part of the C++ API? For example: http://mesos.apache.org/api/latest/c++/namespacemesos.html That way, we don't have to say internal C++ API in the framework development section, which seems a bit odd? Also, can we have the top-level link you have here in the 'contributing to mesos' section? Great suggestions! Will get that in. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35037/#review86510 --- On June 3, 2015, 4 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35037/ --- (Updated June 3, 2015, 4 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Added doxygen link to home.md. Diffs - docs/home.md f2d3e32ca8a1b2735849242daaaf1a6201ce2684 Diff: https://reviews.apache.org/r/35037/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/ --- (Updated June 8, 2015, 5:48 p.m.) Review request for Michael Park. Changes --- Fix leak. Fix bug introduced by net::IP refactor. Bugs: MESOS-1913 https://issues.apache.org/jira/browse/MESOS-1913 Repository: mesos Description --- Requires: configure --enable-libevent --enable-libevent-socket --enable-ssl New environment variables: USE_SSL=(0,1) SSL_CERT=(path to certificate) SSL_KEY=(path to key) SSL_VERIFY_CERT=(0,1) SSL_REQUIRE_CERT=(0,1) SSL_CA_DIR=(path to CA directory) SSL_CA_FILE=(path to CA file) TODO: Restrict SSL version more tightly Track down leak in crypto from accept Diffs (updated) - 3rdparty/libprocess/Makefile.am 489ce359f383d819299335cbaa8c95724b0c6ac2 3rdparty/libprocess/include/process/socket.hpp b8c2274de535ac473e49a09165b601c96d3ebe8b 3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 3rdparty/libprocess/src/libevent.cpp fb038597358135a06c1927d079cb7cb09fea7452 3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 3rdparty/libprocess/src/openssl.hpp PRE-CREATION 3rdparty/libprocess/src/openssl.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp d1b4d469a11abc618c1406bce602300dd9793b58 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 Diff: https://reviews.apache.org/r/29406/diff/ Testing --- make check (uses non-ssl socket) benchmarks using ssl sockets master, slave, framework, webui launch with ssl sockets Thanks, Joris Van Remoortere
Re: Review Request 35152: Document and consolidate qdisc handles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35152/#review87049 --- Ship it! Nice comments! Please adjust the comments as I suggested. We can chat if you don't understand what I was talking about. Otherwise, LGTM! src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/35152/#comment139311 for [7] and [10], I would move the arrow at [7] to veth0 src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/35152/#comment139312 For [8] and [9], I would use a double ended arrow -- and move the arrow at [8] to mesos. src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/35152/#comment139309 Kill this line. src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/35152/#comment139308 2 lines above src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/35152/#comment139310 Please adjust the format. Notice that there is a 'script' in the end. - Jie Yu On June 8, 2015, 4:35 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35152/ --- (Updated June 8, 2015, 4:35 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: MESOS-2821 https://issues.apache.org/jira/browse/MESOS-2821 Repository: mesos Description --- Document and consolidate qdisc handles Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 871e9cf1625d96d1feef50edd4081972c097d191 Diff: https://reviews.apache.org/r/35152/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 35152: Document and consolidate qdisc handles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35152/ --- (Updated June 8, 2015, 6:47 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Address reviewer comments. Bugs: MESOS-2821 https://issues.apache.org/jira/browse/MESOS-2821 Repository: mesos Description --- Document and consolidate qdisc handles Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.cpp 871e9cf1625d96d1feef50edd4081972c097d191 Diff: https://reviews.apache.org/r/35152/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 35165: Extend fq_codel to allow parent and handle to be specified at runtime.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35165/ --- (Updated June 8, 2015, 8:47 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Address review comments. Bugs: MESOS-2332 https://issues.apache.org/jira/browse/MESOS-2332 Repository: mesos Description --- Extend fq_codel to allow parent and handle to be specified at runtime. Diffs (updated) - src/linux/routing/queueing/fq_codel.hpp 412af2d7da2c70324234ee75df75c71319b1365a src/linux/routing/queueing/fq_codel.cpp 3db5b9390bb5ae759eb1b7c9d1ac87cfca8d531e src/slave/containerizer/isolators/network/port_mapping.cpp df2de601a883f4643d632204fdb7b779668a377a src/tests/routing_tests.cpp db5b5df48634ff322baf9328fc605b2667b56eed Diff: https://reviews.apache.org/r/35165/diff/ Testing --- make check Thanks, Paul Brett
Review Request 35229: Report per-container metrics for network bandwidth throttling to the slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35229/ --- Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: MESOS-2332 https://issues.apache.org/jira/browse/MESOS-2332 Repository: mesos Description --- Report per-container metrics for network bandwidth throttling to the slave. Diffs - src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc src/slave/containerizer/isolators/network/port_mapping.cpp d2da1a4e96baeac7d1af9a5468f90c2e4c1cb50f Diff: https://reviews.apache.org/r/35229/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87074 --- src/tests/mesos.hpp https://reviews.apache.org/r/35157/#comment139349 The StartSlave overload is growing a bit out of hand. Let's create a JIRA so we get those consolidated somehow. Can you add the ticket as a comment? :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139350 Can't you compare the message objects instead? If there isn't a == operator overload - let's add one :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139381 Can you inline this below? (Is it used other places?) If not, should we const it? Here and below src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139382 Have you looked into, whether you can use createTask() instead? Here and below src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139380 Why not await ready? Here and below :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139370 const? src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139372 We tend to use the shorter form, if there is no ambguity (controller instead of 'qoSController' :) I'll leave that up to you, if you want to change it src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139369 Let's introduce a 'using std::list;' above :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139375 Why copy the offer? src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139378 Strip 'std::' :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment139377 Let's get the operator overload wired up and do ASSERT_EQ() :) - Niklas Nielsen On June 8, 2015, 12:41 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 8, 2015, 12:41 p.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35207: Included doxygen documentation in docs/home.md.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35207/#review87089 --- Patch looks great! Reviews applied: [35207] All tests passed. - Mesos ReviewBot On June 8, 2015, 8:44 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35207/ --- (Updated June 8, 2015, 8:44 p.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Included doxygen documentation in docs/home.md. Diffs - docs/home.md f2d3e32ca8a1b2735849242daaaf1a6201ce2684 Diff: https://reviews.apache.org/r/35207/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35207: Included doxygen documentation in docs/home.md.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35207/ --- (Updated June 8, 2015, 8:44 p.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Included doxygen documentation in docs/home.md. Diffs - docs/home.md f2d3e32ca8a1b2735849242daaaf1a6201ce2684 Diff: https://reviews.apache.org/r/35207/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35043: WIP: Adding ability to decode JSON from ZK
On June 8, 2015, 8:44 p.m., Alexander Rojas wrote:
src/master/detector.cpp, line 452
https://reviews.apache.org/r/35043/diff/1/?file=978200#file978200line452
Can you add a line break here.
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals
On June 8, 2015, 8:44 p.m., Alexander Rojas wrote:
src/master/detector.cpp, lines 457-458
https://reviews.apache.org/r/35043/diff/1/?file=978200#file978200line457
I think a line break either before or after the declaration of master
info would look better.
Google Style Guide frowns upon too much vertical space - and I do too :)
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Vertical_Whitespace
On June 8, 2015, 8:44 p.m., Alexander Rojas wrote:
src/master/detector.cpp, line 462
https://reviews.apache.org/r/35043/diff/1/?file=978200#file978200line462
A line break here would look better.
sorry, did you mean a blank line above or breaking the `if` and the brace ({)?
- Marco
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35043/#review87079
---
On June 4, 2015, 4:27 a.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35043/
---
(Updated June 4, 2015, 4:27 a.m.)
Review request for mesos, Isabel Jimenez, Joris Van Remoortere, and Vinod
Kone.
Bugs: MESOS-2340
https://issues.apache.org/jira/browse/MESOS-2340
Repository: mesos
Description
---
Jira: MESOS-2340
The Leader detector will now also try to
read nodes with label master::MASTER_INFO_JSON_LABEL
and parse the contents as JSON, converting the data
to a mesos::MasterInfo protobuf.
The ability to support binary data in PB format is
still supported and the master::Contender will continue
to serialize the binary data, so as to be compatible with
pre-0.23 Mesos Masters.
From 0.24 onwards, we will only support JSON data written to ZK.
Diffs
-
src/master/constants.hpp 57cf8fbeff203847b5b5442f6c78ca9c09bcc66d
src/master/constants.cpp 8c7174a9940bd332832bf85d81ab13cf11836dd0
src/master/detector.cpp 5700711771480f4e5da88e60657618c955f10048
Diff: https://reviews.apache.org/r/35043/diff/
Testing
---
make check
(this is still WIP - need to add unit tests for the new JSON format)
Also see manual testing results in
https://issues.apache.org/jira/browse/MESOS-2340
Thanks,
Marco Massenzio
Re: Review Request 35206: Added mainpage to doxygen documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/#review87093 --- Patch looks great! Reviews applied: [35206] All tests passed. - Mesos ReviewBot On June 8, 2015, 8:51 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/ --- (Updated June 8, 2015, 8:51 p.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Added mainpage to doxygen documentation. Diffs - Doxyfile cdc4c12b33a6182ba5f1226b87a47ca491ee5174 src/main.dox PRE-CREATION Diff: https://reviews.apache.org/r/35206/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35225: Add HTB queueing discipline wrapper class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35225/ --- (Updated June 8, 2015, 10:58 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Implement reviewers comment Bugs: MESOS-2752 https://issues.apache.org/jira/browse/MESOS-2752 Repository: mesos Description --- Add HTB queueing discipline wrapper class Diffs (updated) - src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 src/linux/routing/queueing/htb.hpp PRE-CREATION src/linux/routing/queueing/htb.cpp PRE-CREATION src/tests/routing_tests.cpp 4be9967d38177d9f7def5c0da98d4c4266e7f0b5 Diff: https://reviews.apache.org/r/35225/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35206: Added mainpage to doxygen documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/ --- (Updated June 8, 2015, 8:42 p.m.) Review request for Benjamin Hindman and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Added mainpage to doxygen documentation. Diffs (updated) - Doxyfile cdc4c12b33a6182ba5f1226b87a47ca491ee5174 src/main.dox PRE-CREATION Diff: https://reviews.apache.org/r/35206/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35206: Added mainpage to doxygen documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/ --- (Updated June 8, 2015, 8:51 p.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Added mainpage to doxygen documentation. Diffs - Doxyfile cdc4c12b33a6182ba5f1226b87a47ca491ee5174 src/main.dox PRE-CREATION Diff: https://reviews.apache.org/r/35206/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35225: Add HTB queueing discipline wrapper class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35225/#review87088 --- src/linux/routing/queueing/htb.cpp https://reviews.apache.org/r/35225/#comment139365 These params are for fq_codel, doesn't apply for htb. - Cong Wang On June 8, 2015, 9:46 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35225/ --- (Updated June 8, 2015, 9:46 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: MESOS-2752 https://issues.apache.org/jira/browse/MESOS-2752 Repository: mesos Description --- Add HTB queueing discipline wrapper class Diffs - src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 src/linux/routing/queueing/htb.hpp PRE-CREATION src/linux/routing/queueing/htb.cpp PRE-CREATION src/tests/routing_tests.cpp 4be9967d38177d9f7def5c0da98d4c4266e7f0b5 Diff: https://reviews.apache.org/r/35225/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35118: Made updateSlave() update its 'totalResources'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35118/#review87083 --- Ship it! Ship It! - Vinod Kone On June 5, 2015, 9: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, 9: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
Review Request 35225: Add HTB queueing discipline wrapper class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35225/ --- Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: MESOS-2752 https://issues.apache.org/jira/browse/MESOS-2752 Repository: mesos Description --- Add HTB queueing discipline wrapper class Diffs - src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 src/linux/routing/queueing/htb.hpp PRE-CREATION src/linux/routing/queueing/htb.cpp PRE-CREATION src/tests/routing_tests.cpp 4be9967d38177d9f7def5c0da98d4c4266e7f0b5 Diff: https://reviews.apache.org/r/35225/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35037: Added doxygen link to home.md.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35037/ --- (Updated June 8, 2015, 4:41 p.m.) Review request for mesos and Ben Mahler. Changes --- Addressed Ben's comments Repository: mesos Description --- Added doxygen link to home.md. Diffs (updated) - docs/home.md f2d3e32ca8a1b2735849242daaaf1a6201ce2684 Diff: https://reviews.apache.org/r/35037/diff/ Testing --- Thanks, Niklas Nielsen
Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33296/#review87099
---
src/master/main.cpp
https://reviews.apache.org/r/33296/#comment139395
Ditto the comments on slave/main.cpp
src/messages/flags.hpp
https://reviews.apache.org/r/33296/#comment139391
Looks like this comment is just repeating the line of code below it?
src/messages/flags.hpp
https://reviews.apache.org/r/33296/#comment139392
Missing include for ostream?
src/slave/main.cpp
https://reviews.apache.org/r/33296/#comment139394
Would it be clearer to do s/rules/firewall/ and s/_rules/rules/ below?
src/slave/main.cpp
https://reviews.apache.org/r/33296/#comment139397
Missing include for vector?
Missing include for Owned? Also, don't you need a using clause?
Since this is a .cpp file, can you add a using clause for vector to avoid
the std:: prefixing?
src/slave/main.cpp
https://reviews.apache.org/r/33296/#comment139393
Any reason you can't use foreach here?
```
foreach (const string path, rules.disabled_endpoints().paths()) {
paths.insert(path);
}
```
src/slave/main.cpp
https://reviews.apache.org/r/33296/#comment139396
Don't you need a utility include for move?
- Ben Mahler
On June 8, 2015, 12:11 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33296/
---
(Updated June 8, 2015, 12:11 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till
Toenshoff.
Bugs: MESOS-2620
https://issues.apache.org/jira/browse/MESOS-2620
Repository: mesos
Description
---
See summary.
Diffs
-
docs/configuration.md 7d6e78649d0b13b536629bcdfdcb05ff76c7bc54
src/Makefile.am ec7f41f2114d02d3d5a1434d9799f15fe9331917
src/master/flags.hpp 84fa238e5d61731c157b05f7935392dd375d9938
src/master/flags.cpp 49d953a4d2387fa2e28e594988df993762de86df
src/master/main.cpp 3d490c3a5cad59eb908054c3c3872d5540f45b8c
src/messages/flags.hpp PRE-CREATION
src/messages/flags.proto PRE-CREATION
src/slave/flags.hpp 32d36ac57ea7fe16c310fcf8a321312aa42b4b71
src/slave/flags.cpp 1ae106ed59ba54d9cd1aab3f10ceeb8e2c95b19d
src/slave/main.cpp af090ae0dad782a31fc0309b57d8ef474ca74658
Diff: https://reviews.apache.org/r/33296/diff/
Testing
---
make check and manual tests.
Thanks,
Alexander Rojas
Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33295/#review87100
---
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139403
This says connection, but this actually operates at the Request level
AFAICT..?
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139401
The comments after the ';' seem to just re-iterate what the interface
specifies below, so it seems likely to go out of date and I'm not sure it's
adding much value, do you need it?
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139398
Missing include for Try?
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139402
Hm.. I think this interface might prove to be a bit confusing for folks:
```
TryNothing apply = rule.apply(...);
if (apply.isError()) {
// What does this mean?
}
```
What does it mean for there the rule application to fail?
It's subtle, but much like we did for the validation code, it seems clearer
to have the rule application return an optional error. The idea is that rather
than trying to apply and failing, applying the rule always succeeds, but yields
an error (ideally a 'Rejection', but to avoid the unnecessary extra struct):
```
OptionError apply = rule.apply(...);
if (apply.isSome()) {
// This rule rejected the request, when applied.
}
```
Does that make sense?
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139399
Missing include for Error?
3rdparty/libprocess/include/process/firewall.hpp
https://reviews.apache.org/r/33295/#comment139400
Missing include for Nothing?
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139406
newline?
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139414
How about just used to forbid incoming requests.
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139407
Again, this says connections, but these seem to operate at the level of
requests?
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139409
Can you do a sweep for where you're saying connection? Seems to not match
the code :(
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139413
How about the following for the second setence onward:
```
The rules will be applied in the provided order to each incoming request.
If any rule forbids the request, a Forbidden response will be returned
containaing the reason from the rule.
```
3rdparty/libprocess/include/process/process.hpp
https://reviews.apache.org/r/33295/#comment139415
Include for vector?
Include for Owned?
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/33295/#comment139421
We've generally avoided looping using references, although I see why you
did here. Was there a reason that you needed apply to be non-const?
```
foreach (const OwnedFirewallRule rule, firewallRules) {
}
```
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/33295/#comment139420
The convention has been s/applied/apply/ for this kind of code:
```
TryNothing apply = rule-apply(...):
```
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/33295/#comment139416
Newline here?
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139432
Just 'pid' will do :)
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139424
Any plan to add initializer list support for hashset?
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139423
Missing includes for vector, Owned, std::move, can you do a sweep?
Also, can you add a using clause for vector to avoid the std:: prefixing?
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139427
Any reason you could't use initializer list construction for this?
3rdparty/libprocess/src/tests/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139425
Can you scope this 'move' to be safe?
```
{
std::vectorOwnedFirewallRule rules;
rules.emplace_back(new DisabledEndpointsFirewallRule(endpoints));
process::firewall::install(std::move(rules));
}
```
Or avoid it entirely?
```
process::firewall::install(
{ OwnedFirewallRule(new
Re: Review Request 35118: Made updateSlave() update its 'totalResources'.
On June 5, 2015, 9:58 p.m., Vinod Kone wrote: src/master/master.cpp, line 3462 https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3462 woah. didn't realize this was handled automagically by the install handler. Yeah, we didn't do this for framework provided input since the construction to 'Resources' is lossy, and we wanted to validate, but looks good here! Just good to keep in mind that doing this means we cannot do a `Resources::validate` on this. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35118/#review86850 --- On June 5, 2015, 9: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, 9: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 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87118 --- Patch looks great! Reviews applied: [33752, 35084, 35234] All tests passed. - Mesos ReviewBot On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35239: Update the JSON model for Resources to display their revocablility attribute.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35239/#review87121
---
Patch looks great!
Reviews applied: [35239]
All tests passed.
- Mesos ReviewBot
On June 9, 2015, 12:38 a.m., Jiang Yan Xu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35239/
---
(Updated June 9, 2015, 12:38 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-2776
https://issues.apache.org/jira/browse/MESOS-2776
Repository: mesos
Description
---
See summary.
Diffs
-
src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a
src/master/http.cpp f8ac30934352db859e73819e0656a70047bb0dc5
src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7
Diff: https://reviews.apache.org/r/35239/diff/
Testing
---
Added a test.
Also tested with a master/slave pair.
e.g., The following state.json output shows a slave using a fixed estimator
with `cpus:1;mem:512` revocable resources.
```json
slaves: [
{
...
resources: {
cpus: 24,
disk: 454767,
mem: 71322,
ports: [31000-32000]
},
total_resources: {
cpus: 24,
cpus{REV}: 1,
disk: 454767,
mem: 71322,
mem{REV}: 512,
ports: [31000-32000]
},
used_resources: {
cpus: 0,
disk: 0,
mem: 0
}
}
],
```
Note that `resources` only looks at the resources from SlaveInfo while
`total_resources` reads Master::Slave::totalResources.
Thanks,
Jiang Yan Xu
Re: Review Request 35084: Provided consistent behavior for bundled packages.
On June 9, 2015, 12:28 a.m., Cody Maloney wrote: Partial review, going to review more thoroughly later, just posting so it doesn't get lost since it was on the old revision - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35084/#review86739 --- On June 9, 2015, 12:02 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35084/ --- (Updated June 9, 2015, 12:02 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Add the MESOS_USE_BUNDLED_PACKAGE() macro to make it easy to provide consistent behavior for bundled packages selected by either --enable-bundled-$PACKAGE or --with-$PACKAGE. The default policy is set by --enable-bundled and overridden when the user specifies an --enable-bundled-$PACKAGE or --with-$PACKAGE option. If --with-$PACKAGE is specified as bundled, the bundled version is selected. Diffs - configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 Diff: https://reviews.apache.org/r/35084/diff/ Testing --- Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify various (not exhaustive!) combinations of enabling and disableing bundled packages. For example, on CentOS, this alost works: $ onfigure.developer --disable-bundled --with-zookeeper=bundled --with-gmock=bundled To work completely, this change needs to be propagated to libprocess, which I can do once reviewers agree that it's the right behavior. Thanks, James Peach
Re: Review Request 35164: Added callback to the QoS Controller to retrieve usages from the monitor.
On June 9, 2015, 12:34 a.m., Jie Yu wrote:
I think the interface of getting resource usage is going to change per
discussion here:
https://issues.apache.org/jira/browse/MESOS-2818
Niklas Nielsen wrote:
The new proposal doesn't mention changing the callback, does it?
The new protobuf message is what needed by the resource estimator:
```
message ResourceUsage {
message Executor {
optional ExecutorInfo executor_info = 1;
repeated Resource allocated = 2;
repeated ResourceStatistics statistics = 3;
}
repeated Resource total = 1; // Slave's total resources.
repeated Executor executors = 2; // Per-executor allocated/usage information.
}
```
So we'll need to change the interface accordingly.
- Jie
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35164/#review87106
---
On June 5, 2015, 11:44 p.m., Bartek Plotka wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35164/
---
(Updated June 5, 2015, 11:44 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod
Kone.
Bugs: MESOS-2823
https://issues.apache.org/jira/browse/MESOS-2823
Repository: mesos
Description
---
Passed callback to the QoS Controller to retrieve ResourceUsage from Resource
Monitor on demand.
This is neccessary, since QoS Controller needs data (current statistics for
each executor) on which it will base its potential corrections.
Diffs
-
include/mesos/slave/qos_controller.hpp
1d89acfd9c742b044674e0a0815f9f01eccb69b3
src/slave/qos_controller.hpp b37798303561eb79aee202b9c110794517eeed06
src/slave/qos_controller.cpp 81c4b3e658902be0438f42d9e86911e424828a73
src/slave/slave.cpp 054929b156374a8929ac9fffb032045f13c3eb43
src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417
Diff: https://reviews.apache.org/r/35164/diff/
Testing
---
make check
Thanks,
Bartek Plotka
Re: Review Request 35229: Report per-container metrics for network bandwidth throttling to the slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35229/#review87098 --- Patch looks great! Reviews applied: [35225, 35229] All tests passed. - Mesos ReviewBot On June 8, 2015, 10:26 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35229/ --- (Updated June 8, 2015, 10:26 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: MESOS-2332 https://issues.apache.org/jira/browse/MESOS-2332 Repository: mesos Description --- Report per-container metrics for network bandwidth throttling to the slave. Diffs - src/slave/containerizer/isolators/network/port_mapping.hpp 4c719b186b519fad0c3869dbdae8b60c3a2c20cc src/slave/containerizer/isolators/network/port_mapping.cpp d2da1a4e96baeac7d1af9a5468f90c2e4c1cb50f Diff: https://reviews.apache.org/r/35229/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35164: Added callback to the QoS Controller to retrieve usages from the monitor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35164/#review87106 --- I think the interface of getting resource usage is going to change per discussion here: https://issues.apache.org/jira/browse/MESOS-2818 - Jie Yu On June 5, 2015, 11:44 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35164/ --- (Updated June 5, 2015, 11:44 p.m.) Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Bugs: MESOS-2823 https://issues.apache.org/jira/browse/MESOS-2823 Repository: mesos Description --- Passed callback to the QoS Controller to retrieve ResourceUsage from Resource Monitor on demand. This is neccessary, since QoS Controller needs data (current statistics for each executor) on which it will base its potential corrections. Diffs - include/mesos/slave/qos_controller.hpp 1d89acfd9c742b044674e0a0815f9f01eccb69b3 src/slave/qos_controller.hpp b37798303561eb79aee202b9c110794517eeed06 src/slave/qos_controller.cpp 81c4b3e658902be0438f42d9e86911e424828a73 src/slave/slave.cpp 054929b156374a8929ac9fffb032045f13c3eb43 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 Diff: https://reviews.apache.org/r/35164/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87115 --- Patch looks great! Reviews applied: [35164, 35157] All tests passed. - Mesos ReviewBot On June 9, 2015, 12:17 a.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 9, 2015, 12:17 a.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35084: Provided consistent behavior for bundled packages.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35084/#review86739 --- configure.ac https://reviews.apache.org/r/35084/#comment138798 nit: Could you document auto here? Right now it's an implicit magic value like 'bundled' in a lot of ways. I also wonder if it is possible to have this macro set the AC_ARG_WITH / AC_ARG_ENABLE, but that definitely isn't necessary / blocker here. configure.ac https://reviews.apache.org/r/35084/#comment138799 Since these all follow about the same structure could we add an autoconf macro which takes the package name followed by the help description and does the rest automatically? - Cody Maloney On June 9, 2015, 12:02 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35084/ --- (Updated June 9, 2015, 12:02 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Add the MESOS_USE_BUNDLED_PACKAGE() macro to make it easy to provide consistent behavior for bundled packages selected by either --enable-bundled-$PACKAGE or --with-$PACKAGE. The default policy is set by --enable-bundled and overridden when the user specifies an --enable-bundled-$PACKAGE or --with-$PACKAGE option. If --with-$PACKAGE is specified as bundled, the bundled version is selected. Diffs - configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 Diff: https://reviews.apache.org/r/35084/diff/ Testing --- Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify various (not exhaustive!) combinations of enabling and disableing bundled packages. For example, on CentOS, this alost works: $ onfigure.developer --disable-bundled --with-zookeeper=bundled --with-gmock=bundled To work completely, this change needs to be propagated to libprocess, which I can do once reviewers agree that it's the right behavior. Thanks, James Peach
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 256 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line256 Why not await ready? Here and below :) hmm because when we receive status 'running' from task, then we are pretty sure that Resource Estimator/QoS Controller is initialized (: or rather.. we want to make sure (: On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/mesos.hpp, lines 188-201 https://reviews.apache.org/r/35157/diff/3/?file=980316#file980316line188 The StartSlave overload is growing a bit out of hand. Let's create a JIRA so we get those consolidated somehow. Can you add the ticket as a comment? :) MESOS-2833 On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 209 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line209 Can you inline this below? (Is it used other places?) If not, should we const it? Here and below Added inline if possible, otherwise const. Added const also for the other variables which are not being changed during test. On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, lines 234-238 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line234 Have you looked into, whether you can use createTask() instead? Here and below Good idea - mimized from e.g master_test. On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 550 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line550 We tend to use the shorter form, if there is no ambguity (controller instead of 'qoSController' :) I'll leave that up to you, if you want to change it +1 Done. On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 552 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line552 Let's introduce a 'using std::list;' above :) Actually - it was (: But i did not realize that, thx. On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 618 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line618 Let's get the operator overload wired up and do ASSERT_EQ() :) Done - in type_utils - Bartek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87074 --- On June 9, 2015, 12:17 a.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 9, 2015, 12:17 a.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Review Request 35239: Update the JSON model for Resources to display their revocablility attribute.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35239/
---
Review request for mesos and Vinod Kone.
Bugs: MESOS-2776
https://issues.apache.org/jira/browse/MESOS-2776
Repository: mesos
Description
---
See summary.
Diffs
-
src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a
src/master/http.cpp f8ac30934352db859e73819e0656a70047bb0dc5
src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7
Diff: https://reviews.apache.org/r/35239/diff/
Testing
---
Added a test.
Also tested with a master/slave pair.
e.g., The following state.json output shows a slave using a fixed estimator
with `cpus:1;mem:512` revocable resources.
```json
slaves: [
{
...
resources: {
cpus: 24,
disk: 454767,
mem: 71322,
ports: [31000-32000]
},
total_resources: {
cpus: 24,
cpus{REV}: 1,
disk: 454767,
mem: 71322,
mem{REV}: 512,
ports: [31000-32000]
},
used_resources: {
cpus: 0,
disk: 0,
mem: 0
}
}
],
```
Note that `resources` only looks at the resources from SlaveInfo while
`total_resources` reads Master::Slave::totalResources.
Thanks,
Jiang Yan Xu
Re: Review Request 35164: Added callback to the QoS Controller to retrieve usages from the monitor.
On June 8, 2015, 5:34 p.m., Jie Yu wrote: I think the interface of getting resource usage is going to change per discussion here: https://issues.apache.org/jira/browse/MESOS-2818 The new proposal doesn't mention changing the callback, does it? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35164/#review87106 --- On June 5, 2015, 4:44 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35164/ --- (Updated June 5, 2015, 4:44 p.m.) Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Bugs: MESOS-2823 https://issues.apache.org/jira/browse/MESOS-2823 Repository: mesos Description --- Passed callback to the QoS Controller to retrieve ResourceUsage from Resource Monitor on demand. This is neccessary, since QoS Controller needs data (current statistics for each executor) on which it will base its potential corrections. Diffs - include/mesos/slave/qos_controller.hpp 1d89acfd9c742b044674e0a0815f9f01eccb69b3 src/slave/qos_controller.hpp b37798303561eb79aee202b9c110794517eeed06 src/slave/qos_controller.cpp 81c4b3e658902be0438f42d9e86911e424828a73 src/slave/slave.cpp 054929b156374a8929ac9fffb032045f13c3eb43 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 Diff: https://reviews.apache.org/r/35164/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35120: Use non-POD type for alias example in c++ style guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35120/#review87113 --- docs/mesos-c++-style-guide.md https://reviews.apache.org/r/35120/#comment139436 By the way, I removed the std:: prefix here. docs/mesos-c++-style-guide.md https://reviews.apache.org/r/35120/#comment139437 Also I added const to the 'values', was that not the intent? - Ben Mahler On June 5, 2015, 9:34 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35120/ --- (Updated June 5, 2015, 9:34 a.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Repository: mesos Description --- Also got rid of an unused struct. Diffs - docs/mesos-c++-style-guide.md 38dd201a22dd775971fad190378d022c50885969 Diff: https://reviews.apache.org/r/35120/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 34921: Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34921/#review87117 --- src/cli/mesos-ps https://reviews.apache.org/r/34921/#comment139443 Do you still need the fraction on 1024.0 with this? Maybe a note is needed: ``` // Ensure bytes is treated as floating point for the math below. bytes = float(bytes) ... ``` - Ben Mahler On June 2, 2015, 3:28 a.m., weitao zhou wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34921/ --- (Updated June 2, 2015, 3:28 a.m.) Review request for mesos. Repository: mesos Description --- Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize. Diffs - src/cli/mesos-ps 8ca7a64d0ab6f0b8db823ca95d5f7500487b8753 Diff: https://reviews.apache.org/r/34921/diff/ Testing --- Thanks, weitao zhou
Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33752/ --- (Updated June 9, 2015, 12:02 a.m.) Review request for mesos, Cody Maloney and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- In a number of places, the Mesos configure script passes $foo=yes to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument is invoked when the option is provided in any form, not just when the --enable-foo form is used. One result of this is that --disable-optimize doesn't work. The correct handling of the 2nd argument is to save the value of $enableval. This change sets the value of all the enable variables using $enableval, and sets the default value based on the option name. There are a number of enable options that were internally named $with_foo and $without_foo. Rename these to $enable_foo for clarity and to remove the need for both a with_ and a without_ version. Finally, emit the compilation flags at the end of the configure phase so it is easier to see the results of your configuration options. Diffs (updated) - configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 Diff: https://reviews.apache.org/r/33752/diff/ Testing --- Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things. Thanks, James Peach
Re: Review Request 35084: Provided consistent behavior for bundled packages.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35084/ --- (Updated June 9, 2015, 12:02 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Add the MESOS_USE_BUNDLED_PACKAGE() macro to make it easy to provide consistent behavior for bundled packages selected by either --enable-bundled-$PACKAGE or --with-$PACKAGE. The default policy is set by --enable-bundled and overridden when the user specifies an --enable-bundled-$PACKAGE or --with-$PACKAGE option. If --with-$PACKAGE is specified as bundled, the bundled version is selected. Diffs (updated) - configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 Diff: https://reviews.apache.org/r/35084/diff/ Testing --- Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify various (not exhaustive!) combinations of enabling and disableing bundled packages. For example, on CentOS, this alost works: $ onfigure.developer --disable-bundled --with-zookeeper=bundled --with-gmock=bundled To work completely, this change needs to be propagated to libprocess, which I can do once reviewers agree that it's the right behavior. Thanks, James Peach
Re: Review Request 34258: Removed os::dirname and os::basename.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34258/ --- (Updated June 9, 2015, 12:01 a.m.) Review request for mesos and Cody Maloney. Changes --- re-triggering the buildbot Bugs: MESOS-1303 https://issues.apache.org/jira/browse/MESOS-1303 Repository: mesos-incubating Description --- os::dirname and os::basename were not thread safe on OSX. Replacements are path::dirname and path::basename. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/README.md 588f739 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp d2ca4be Diff: https://reviews.apache.org/r/34258/diff/ Testing --- make check on trailing RR. Thanks, Till Toenshoff
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87101 --- Great updates - quick style review which applies to the dependending one as well. 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139405 Can we switch to `#` prefixed comments here instead? 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139412 s/packaged/package/g 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139408 Please end comments with a punctuation. - Till Toenshoff On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 582 https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line582 Why copy the offer? Because we use it several time, however i didn't notice that it wasn't consistent - sometimes i used *offer*, sometimes *offer.get()[0]*. Fixed now. - Bartek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87074 --- On June 9, 2015, 12:17 a.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 9, 2015, 12:17 a.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35120: Use non-POD type for alias example in c++ style guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35120/#review87111 --- Ship it! Ship It! - Ben Mahler On June 5, 2015, 9:34 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35120/ --- (Updated June 5, 2015, 9:34 a.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Repository: mesos Description --- Also got rid of an unused struct. Diffs - docs/mesos-c++-style-guide.md 38dd201a22dd775971fad190378d022c50885969 Diff: https://reviews.apache.org/r/35120/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 9, 2015, 12:17 a.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Changes --- Issues addressed. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs (updated) - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 32999: Added a document for engineering principles and practices.
On June 2, 2015, 3:19 p.m., Niklas Nielsen wrote: Ping @benm :) Do you want this in? Thanks, I'll land it now. Kept pinging benh to take a look but let's follow up if he has feedback, since there seems to be consensus around these. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32999/#review86224 --- On April 23, 2015, 11:15 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32999/ --- (Updated April 23, 2015, 11:15 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Jie Yu, Niklas Nielsen, and Vinod Kone. Repository: mesos Description --- Added a document for engineering principles and practices. Diffs - docs/engineering-principles-and-practices.md PRE-CREATION docs/home.md 641ee8f5e7cc2f9ccd62a5c4236912886aaa7a1d Diff: https://reviews.apache.org/r/32999/diff/ Testing --- N/A Thanks, Ben Mahler
Re: Review Request 34921: Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34921/ --- (Updated 六月 9, 2015, 2:58 a.m.) Review request for mesos. Repository: mesos Description --- Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize. Diffs (updated) - src/cli/mesos-ps ae48c8cde31e90024381ed3e6e22ec3bc96560b6 Diff: https://reviews.apache.org/r/34921/diff/ Testing --- Thanks, weitao zhou
Re: Review Request 33057: Added secret check to CRAM-MD5 authenticatee.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33057/#review87009 --- Patch looks great! Reviews applied: [32850, 33057] All tests passed. - Mesos ReviewBot On June 8, 2015, 9:42 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33057/ --- (Updated June 8, 2015, 9:42 a.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2608 https://issues.apache.org/jira/browse/MESOS-2608 Repository: mesos-incubating Description --- Updating authenticatee to check for secret within credential. Adding a test verifying immediate authenticatee failure when secret is missing. Diffs - src/authentication/cram_md5/authenticatee.cpp PRE-CREATION src/tests/cram_md5_authentication_tests.cpp 9923023 Diff: https://reviews.apache.org/r/33057/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 33057: Added secret check to CRAM-MD5 authenticatee.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33057/ --- (Updated June 8, 2015, 9:42 a.m.) Review request for mesos, Adam B and Vinod Kone. Changes --- Reupped to retrigger buildbot. Bugs: MESOS-2608 https://issues.apache.org/jira/browse/MESOS-2608 Repository: mesos-incubating Description --- Updating authenticatee to check for secret within credential. Adding a test verifying immediate authenticatee failure when secret is missing. Diffs (updated) - src/authentication/cram_md5/authenticatee.cpp PRE-CREATION src/tests/cram_md5_authentication_tests.cpp 9923023 Diff: https://reviews.apache.org/r/33057/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 34703: Added stream manipulators for the Time object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review87028 --- 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139287 Move the opening brace to the first line please (like you did below). 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139286 Lets make this a copy instead to prevent weird bugs introduced by taking references to temporals. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139288 `const Time time;` - Till Toenshoff On May 29, 2015, 12:59 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/ --- (Updated May 29, 2015, 12:59 p.m.) Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- Adds some manipulator classes which allows formatting Time objects to ostreams. Diffs - 3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9 3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b Diff: https://reviews.apache.org/r/34703/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/ --- (Updated June 8, 2015, 2:11 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Changes --- rebased Bugs: MESOS-2620 https://issues.apache.org/jira/browse/MESOS-2620 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md 7d6e78649d0b13b536629bcdfdcb05ff76c7bc54 src/Makefile.am ec7f41f2114d02d3d5a1434d9799f15fe9331917 src/master/flags.hpp 84fa238e5d61731c157b05f7935392dd375d9938 src/master/flags.cpp 49d953a4d2387fa2e28e594988df993762de86df src/master/main.cpp 3d490c3a5cad59eb908054c3c3872d5540f45b8c src/messages/flags.hpp PRE-CREATION src/messages/flags.proto PRE-CREATION src/slave/flags.hpp 32d36ac57ea7fe16c310fcf8a321312aa42b4b71 src/slave/flags.cpp 1ae106ed59ba54d9cd1aab3f10ceeb8e2c95b19d src/slave/main.cpp af090ae0dad782a31fc0309b57d8ef474ca74658 Diff: https://reviews.apache.org/r/33296/diff/ Testing --- make check and manual tests. Thanks, Alexander Rojas
Re: Review Request 34703: Added stream manipulators for the Time object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review87013 --- Looks pretty good. I just am unsure about the class naming - the rest is just nits. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139262 Can we give this a different, more intuitive name please? I would suggest `HTTPDate` instead. Can we please also add an example so people looking for a particular format will quickly find it? 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139263 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139264 `snprintf` does not seem to set `errno`, hence `LOG` should be used instead of `PLOG`. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139265 2 chars indent please. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139275 Lets think about a good replacement for that class name to make it intuitive - first thing that came to mind is ```UnixTimestamp```. Lets also add an example right here in this comment. 3rdparty/libprocess/include/process/time.hpp https://reviews.apache.org/r/34703/#comment139276 2 chars intention please. - Till Toenshoff On May 29, 2015, 12:59 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/ --- (Updated May 29, 2015, 12:59 p.m.) Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- Adds some manipulator classes which allows formatting Time objects to ostreams. Diffs - 3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9 3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b Diff: https://reviews.apache.org/r/34703/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/ --- (Updated June 8, 2015, 12:58 p.m.) Review request for Michael Park. Changes --- fixed issues. rebased. Bugs: MESOS-1913 https://issues.apache.org/jira/browse/MESOS-1913 Repository: mesos Description --- Requires: configure --enable-libevent --enable-libevent-socket --enable-ssl New environment variables: USE_SSL=(0,1) SSL_CERT=(path to certificate) SSL_KEY=(path to key) SSL_VERIFY_CERT=(0,1) SSL_REQUIRE_CERT=(0,1) SSL_CA_DIR=(path to CA directory) SSL_CA_FILE=(path to CA file) TODO: Restrict SSL version more tightly Track down leak in crypto from accept Diffs (updated) - 3rdparty/libprocess/Makefile.am 489ce359f383d819299335cbaa8c95724b0c6ac2 3rdparty/libprocess/include/process/socket.hpp b8c2274de535ac473e49a09165b601c96d3ebe8b 3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 3rdparty/libprocess/src/libevent.cpp fb038597358135a06c1927d079cb7cb09fea7452 3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 3rdparty/libprocess/src/openssl.hpp PRE-CREATION 3rdparty/libprocess/src/openssl.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp d1b4d469a11abc618c1406bce602300dd9793b58 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 Diff: https://reviews.apache.org/r/29406/diff/ Testing --- make check (uses non-ssl socket) benchmarks using ssl sockets master, slave, framework, webui launch with ssl sockets Thanks, Joris Van Remoortere
Re: Review Request 33730: Fixes template style issue in common/parse.hpp
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33730/ --- (Updated June 8, 2015, 2:06 p.m.) Review request for mesos, Benjamin Hindman, Michael Park, and Till Toenshoff. Repository: mesos Description --- Fixes all versions of `template` to `template ` in order to accommodate to the guidelines. Diffs - Diff: https://reviews.apache.org/r/33730/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/#review87023 --- Patch looks great! Reviews applied: [33295, 33296] All tests passed. - Mesos ReviewBot On June 8, 2015, 12:11 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/ --- (Updated June 8, 2015, 12:11 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-2620 https://issues.apache.org/jira/browse/MESOS-2620 Repository: mesos Description --- See summary. Diffs - docs/configuration.md 7d6e78649d0b13b536629bcdfdcb05ff76c7bc54 src/Makefile.am ec7f41f2114d02d3d5a1434d9799f15fe9331917 src/master/flags.hpp 84fa238e5d61731c157b05f7935392dd375d9938 src/master/flags.cpp 49d953a4d2387fa2e28e594988df993762de86df src/master/main.cpp 3d490c3a5cad59eb908054c3c3872d5540f45b8c src/messages/flags.hpp PRE-CREATION src/messages/flags.proto PRE-CREATION src/slave/flags.hpp 32d36ac57ea7fe16c310fcf8a321312aa42b4b71 src/slave/flags.cpp 1ae106ed59ba54d9cd1aab3f10ceeb8e2c95b19d src/slave/main.cpp af090ae0dad782a31fc0309b57d8ef474ca74658 Diff: https://reviews.apache.org/r/33296/diff/ Testing --- make check and manual tests. Thanks, Alexander Rojas
Re: Review Request 35206: Added mainpage to doxygen documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/ --- (Updated June 8, 2015, 4:22 p.m.) Review request for mesos and Bernd Mathiske. Changes --- Addressed comments Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Added mainpage to doxygen documentation. Diffs (updated) - Doxyfile cdc4c12b33a6182ba5f1226b87a47ca491ee5174 src/main.dox PRE-CREATION Diff: https://reviews.apache.org/r/35206/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 35206: Added mainpage to doxygen documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/#review87045 --- Ship it! Ship It! - Bernd Mathiske On June 8, 2015, 9:22 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35206/ --- (Updated June 8, 2015, 9:22 a.m.) Review request for mesos and Bernd Mathiske. Bugs: Mesos-2500 https://issues.apache.org/jira/browse/Mesos-2500 Repository: mesos Description --- Added mainpage to doxygen documentation. Diffs - Doxyfile cdc4c12b33a6182ba5f1226b87a47ca491ee5174 src/main.dox PRE-CREATION Diff: https://reviews.apache.org/r/35206/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review87041 --- Patch looks great! Reviews applied: [34703, 30032] All tests passed. - Mesos ReviewBot On June 8, 2015, 3:14 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated June 8, 2015, 3:14 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Diffs - 3rdparty/libprocess/include/process/http.hpp 51a00f5cf6a84413c8ab73a4e62e1814e9af2339 3rdparty/libprocess/src/process.cpp aadd7bb0ae12b93336900c76d8d5aaa4421ea198 3rdparty/libprocess/src/tests/process_tests.cpp 0832c6266cea0fb6bdbd523921c1367959419af8 Diff: https://reviews.apache.org/r/30032/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 35152: Document and consolidate qdisc handles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35152/ --- (Updated June 8, 2015, 4:35 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Incorporate review comments. Bugs: MESOS-2821 https://issues.apache.org/jira/browse/MESOS-2821 Repository: mesos Description --- Document and consolidate qdisc handles Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.cpp 871e9cf1625d96d1feef50edd4081972c097d191 Diff: https://reviews.apache.org/r/35152/diff/ Testing --- make check Thanks, Paul Brett
