Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/#review50396 --- Ship it! Modulo Vinod's comments. Looks good otherwise! src/master/master.cpp https://reviews.apache.org/r/24576/#comment88172 It this comment still relevant? - Niklas Nielsen On Aug. 11, 2014, 4:32 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 11, 2014, 4:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp e688b41b9f2e555acd8fe0da5d3eb4e8bce32211 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24808: Modify execute cli to run docker image
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24808/#review50920 --- Ship it! Looks great! Looking forward to use this :-) src/cli/execute.cpp https://reviews.apache.org/r/24808/#comment88803 How about leaving an example? It is the docker:/// format, right? - Niklas Nielsen On Aug. 18, 2014, 11:29 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24808/ --- (Updated Aug. 18, 2014, 11:29 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Modify execute cli to run docker image Diffs - src/cli/execute.cpp 7e20f4c54f15047eb73948ca7eeac425160a8a85 Diff: https://reviews.apache.org/r/24808/diff/ Testing --- make Tested it manually as well. Thanks, Timothy Chen
Re: [VOTE] Release Apache Mesos 0.20.0 (rc2)
+1 make check on Ubuntu 14.04 gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1) and Mac OS X clang-503.0.40 Looking so much forward to this release! Niklas On 19 August 2014 11:28, Ian Downes idow...@twitter.com.invalid wrote: +1 make check passes on CentOS 5.5 with 3.4 kernel. Docker tests were ignored. On Aug 19, 2014, at 10:54 AM, Vinod Kone vinodk...@gmail.com wrote: +1 make check passes on OSX Mavericks and CentOS 5.5 On Mon, Aug 18, 2014 at 11:26 PM, Jie Yu yujie@gmail.com wrote: Hi all, Please vote on releasing the following candidate as Apache Mesos 0.20.0. NOTE: 0.20.0-rc1 has a bug on Mac (MESOS-1713) which is fixed in 0.20.0-rc2. 0.20.0 includes the following: This release includes a lot of new cool features. The major new features are listed below: * Docker support in Mesos: * Users now can launch executors/tasks within Docker containers. * Mesos now supports running multiple containerizers simultaneously. The slave can dynamically choose a containerizer to launch containers based on the configuration of executors/tasks. * Container level network monitoring for mesos containerizer: * Network statistics for each active container can be retrieved through the /monitor/statistics.json endpoint on the slave. * Completely transparent to the tasks running on the slave. No need to change the service discovery mechanism for tasks. * Framework authorization: * Allows frameworks to (re-)register with authorized roles. * Allows frameworks to launch tasks/executors as authorized users. * Allows authorized principals to shutdown framework(s) through HTTP endpoint. * Framework rate limiting: * In a multi-framework environment, this feature aims to protect the throughput of high-SLA (e.g., production, service) frameworks by having the master throttle messages from other (e.g., development, batch) frameworks. * Enable building against installed third-party dependencies. * API Changes: * [MESOS-857] - The Python API now uses different namespacing. This will break existing schedulers, please refer to the upgrades document. * [MESOS-1409] - Status update acknowledgements are sent through the Master now. This only affects you if you're using a non-Mesos binding (e.g. pure language binding), in which case refer to the upgrades document. * HTTP endpoint changes: * [MESOS-1188] - deactivated_slaves represents inactive slaves in /stats.json and /state.json. * [MESOS-1390] - /shutdown authenticated endpoint has been added to master to shutdown a framework. * Deprecations: * [MESOS-1219] - Master should disallow completed frameworks from re-registering with same framework id. * [MESOS-1695] - /stats.json on the slave exposes registered value as string instead of integer. This release also includes several bug fixes and stability improvements. The candidate for Mesos 0.20.0 release is available at: https://dist.apache.org/repos/dist/dev/mesos/0.20.0-rc2/mesos-0.20.0.tar.gz The tag to be voted on is 0.20.0-rc2: https://git-wip-us.apache.org/repos/asf?p=mesos.git;a=commit;h=0.20.0-rc2 The MD5 checksum of the tarball can be found at: https://dist.apache.org/repos/dist/dev/mesos/0.20.0-rc2/mesos-0.20.0.tar.gz.md5 The signature of the tarball can be found at: https://dist.apache.org/repos/dist/dev/mesos/0.20.0-rc2/mesos-0.20.0.tar.gz.asc The PGP key used to sign the release is here: https://dist.apache.org/repos/dist/release/mesos/KEYS The JAR is up in Maven in a staging repository here: https://repository.apache.org/content/repositories/orgapachemesos-1030 Please vote on releasing this package as Apache Mesos 0.20.0! The vote is open until Thu Aug 21 23:23:19 PDT 2014 and passes if a majority of at least 3 +1 PMC votes are cast. [ ] +1 Release this package as Apache Mesos 0.20.0 [ ] -1 Do not release this package because ... Thanks, - Jie
Re: new contributor
Hi Jason, I added you to the JIRA Mesos contributor list. Cheers, Niklas On 26 August 2014 14:09, Jason jasonl9...@gmail.com wrote:
Re: new contributor
Patrick, added you already - can you share the jira handles of the other folks? Niklas On 26 August 2014 14:25, Jason jasonl9...@gmail.com wrote: Thanks! On Tue, Aug 26, 2014 at 2:15 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Jason, I added you to the JIRA Mesos contributor list. Cheers, Niklas On 26 August 2014 14:09, Jason jasonl9...@gmail.com wrote:
Re: new contributor
Great - just added them. Niklas On 26 August 2014 14:49, Patrick Reilly patrick.rei...@gmail.com wrote: Hello Niklas, It's jmlvanre, mcypark and cmaloney. — Patrick On Tue, Aug 26, 2014 at 2:27 PM, Niklas Nielsen nik...@mesosphere.io wrote: Patrick, added you already - can you share the jira handles of the other folks? Niklas On 26 August 2014 14:25, Jason jasonl9...@gmail.com wrote: Thanks! On Tue, Aug 26, 2014 at 2:15 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Jason, I added you to the JIRA Mesos contributor list. Cheers, Niklas On 26 August 2014 14:09, Jason jasonl9...@gmail.com wrote:
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 2:58 p.m., Ben Mahler wrote: It was done using macro expansion because variadic templates require C++11. We're not yet able to assume C++11: https://issues.apache.org/jira/browse/MESOS-750 But as this reduce compile time for folks with C++11 compilers, can we make this guarded by #ifdef and enable it for good when we make the transition? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51595 --- On Aug. 26, 2014, 2:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 2:44 p.m.) Review request for mesos, Adam B and Benjamin Hindman. Bugs: MESOS-1734 https://issues.apache.org/jira/browse/MESOS-1734 Repository: mesos-git Description --- Reduce compile time: - replacing a macro expansion with a variadic template - moving implementation from help.hpp to help.cpp Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/include/process/help.hpp 4333b5b 3rdparty/libprocess/src/help.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25079/diff/ Testing --- Ran make check. Thanks, Patrick Reilly
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 7:36 p.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [25079] Failed command: ./support/mesos-style.py Error: Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo 3rdparty/libprocess/include/process/help.hpp:61: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/include/process/help.hpp:72: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/src/help.cpp:77: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:78: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:124: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:136: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:139: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:154: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 8 Probably not you guys, but can you address these style bugs too? You can run ./support/mesos-style.py to get the style report. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51633 --- On Aug. 26, 2014, 2:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 2:44 p.m.) Review request for mesos, Adam B and Benjamin Hindman. Bugs: MESOS-1734 https://issues.apache.org/jira/browse/MESOS-1734 Repository: mesos-git Description --- Reduce compile time: - replacing a macro expansion with a variadic template - moving implementation from help.hpp to help.cpp Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/include/process/help.hpp 4333b5b 3rdparty/libprocess/src/help.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25079/diff/ Testing --- Ran make check. Thanks, Patrick Reilly
Review Request 25124: Added 'net-bench' libprocess benchmarking tool.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/ --- Review request for mesos, Benjamin Hindman and Ben Mahler. Repository: mesos-git Description --- Added libprocess connection throughput stress test (basically two libprocess programs sending messsages back and forth). One end is multihomed so we can scale the number of clients. One client will issue multiple concurrent messages (messages sent before waiting for acknowledgement). The motivation is to stress test libprocess communication and get an idea on theoretical throughput / scale-properties for increase slave-fanout. Diffs - src/Makefile.am 40b9f6b src/net-bench/main.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25124/diff/ Testing --- Server: $ ./src/mesos-net-bench WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:01.605250 259977216 main.cpp:111] Running server at (1)@10.0.0.224:53492 I0827 15:48:13.617341 259977216 main.cpp:135] 14628.2 requests / second I0827 15:48:14.617578 258367488 main.cpp:135] 13886.7 requests / second I0827 15:48:15.617683 257830912 main.cpp:135] 8677.19 requests / second I0827 15:48:16.617771 257830912 main.cpp:135] 8304.19 requests / second Client: $ ./build/src/mesos-net-bench --server=(1)@10.0.0.224:53492 WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:12.624887 124162048 main.cpp:115] Connecting to server at (1)@10.0.0.224:53492... Thanks, Niklas Nielsen
Re: Review Request 25124: Added 'net-bench' libprocess benchmarking tool.
On Aug. 27, 2014, 4:01 p.m., Dominic Hamon wrote: src/net-bench/main.cpp, line 166 https://reviews.apache.org/r/25124/diff/1/?file=670615#file670615line166 it might be worth (for a future version) allowing this to be a random string of user-defined length and also tracking statistics for bytes sent/received. Good point - we would need to install message handlers accordingly though. I would categorize it as future work :-) Want a TODO in place? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/#review51716 --- On Aug. 27, 2014, 3:49 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/ --- (Updated Aug. 27, 2014, 3:49 p.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Repository: mesos-git Description --- Added libprocess connection throughput stress test (basically two libprocess programs sending messsages back and forth). One end is multihomed so we can scale the number of clients. One client will issue multiple concurrent messages (messages sent before waiting for acknowledgement). The motivation is to stress test libprocess communication and get an idea on theoretical throughput / scale-properties for increase slave-fanout. Diffs - src/Makefile.am 40b9f6b src/net-bench/main.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25124/diff/ Testing --- Server: $ ./src/mesos-net-bench WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:01.605250 259977216 main.cpp:111] Running server at (1)@10.0.0.224:53492 I0827 15:48:13.617341 259977216 main.cpp:135] 14628.2 requests / second I0827 15:48:14.617578 258367488 main.cpp:135] 13886.7 requests / second I0827 15:48:15.617683 257830912 main.cpp:135] 8677.19 requests / second I0827 15:48:16.617771 257830912 main.cpp:135] 8304.19 requests / second Client: $ ./build/src/mesos-net-bench --server=(1)@10.0.0.224:53492 WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:12.624887 124162048 main.cpp:115] Connecting to server at (1)@10.0.0.224:53492... Thanks, Niklas Nielsen
Re: Review Request 25124: Added 'net-bench' libprocess benchmarking tool.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/ --- (Updated Aug. 28, 2014, 11:51 a.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Changes --- Added note on bandwidth measurement. Repository: mesos-git Description --- Added libprocess connection throughput stress test (basically two libprocess programs sending messsages back and forth). One end is multihomed so we can scale the number of clients. One client will issue multiple concurrent messages (messages sent before waiting for acknowledgement). The motivation is to stress test libprocess communication and get an idea on theoretical throughput / scale-properties for increase slave-fanout. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6fa5b74 3rdparty/libprocess/3rdparty/stout/include/stout/glog.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp 30232f5 3rdparty/libprocess/3rdparty/stout/tests/main.cpp f5f20ee 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e src/Makefile.am 40b9f6b src/logging/logging.cpp 6b14575 src/net-bench/main.cpp PRE-CREATION src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/external_containerizer.hpp 8363cec src/slave/containerizer/external_containerizer.cpp efbc68f src/tests/main.cpp 90a7ddc Diff: https://reviews.apache.org/r/25124/diff/ Testing --- Server: $ ./src/mesos-net-bench WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:01.605250 259977216 main.cpp:111] Running server at (1)@10.0.0.224:53492 I0827 15:48:13.617341 259977216 main.cpp:135] 14628.2 requests / second I0827 15:48:14.617578 258367488 main.cpp:135] 13886.7 requests / second I0827 15:48:15.617683 257830912 main.cpp:135] 8677.19 requests / second I0827 15:48:16.617771 257830912 main.cpp:135] 8304.19 requests / second Client: $ ./build/src/mesos-net-bench --server=(1)@10.0.0.224:53492 WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:12.624887 124162048 main.cpp:115] Connecting to server at (1)@10.0.0.224:53492... Thanks, Niklas Nielsen
Re: Review Request 25124: Added 'net-bench' libprocess benchmarking tool.
On Aug. 27, 2014, 4:01 p.m., Dominic Hamon wrote: src/net-bench/main.cpp, line 166 https://reviews.apache.org/r/25124/diff/1/?file=670615#file670615line166 it might be worth (for a future version) allowing this to be a random string of user-defined length and also tracking statistics for bytes sent/received. Niklas Nielsen wrote: Good point - we would need to install message handlers accordingly though. I would categorize it as future work :-) Want a TODO in place? Dominic Hamon wrote: sure. i was also going to suggest using the libprocess metrics library to get access to the counters through an endpoint, but that's almost certainly overkill. as an aside, the numbers you get are similar to some theoretical upper-bound numbers we saw when testing the rate limiting framework. that suggests that 1. your methodology is sound and 2. that libprocess is the bottleneck rather than the communication up the stack to mesos. Cool - just updated the patch with a TODO on bandwidth tracking. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/#review51716 --- On Aug. 28, 2014, 11:51 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/ --- (Updated Aug. 28, 2014, 11:51 a.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Repository: mesos-git Description --- Added libprocess connection throughput stress test (basically two libprocess programs sending messsages back and forth). One end is multihomed so we can scale the number of clients. One client will issue multiple concurrent messages (messages sent before waiting for acknowledgement). The motivation is to stress test libprocess communication and get an idea on theoretical throughput / scale-properties for increase slave-fanout. Diffs - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6fa5b74 3rdparty/libprocess/3rdparty/stout/include/stout/glog.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp 30232f5 3rdparty/libprocess/3rdparty/stout/tests/main.cpp f5f20ee 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e src/Makefile.am 40b9f6b src/logging/logging.cpp 6b14575 src/net-bench/main.cpp PRE-CREATION src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/external_containerizer.hpp 8363cec src/slave/containerizer/external_containerizer.cpp efbc68f src/tests/main.cpp 90a7ddc Diff: https://reviews.apache.org/r/25124/diff/ Testing --- Server: $ ./src/mesos-net-bench WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:01.605250 259977216 main.cpp:111] Running server at (1)@10.0.0.224:53492 I0827 15:48:13.617341 259977216 main.cpp:135] 14628.2 requests / second I0827 15:48:14.617578 258367488 main.cpp:135] 13886.7 requests / second I0827 15:48:15.617683 257830912 main.cpp:135] 8677.19 requests / second I0827 15:48:16.617771 257830912 main.cpp:135] 8304.19 requests / second Client: $ ./build/src/mesos-net-bench --server=(1)@10.0.0.224:53492 WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:12.624887 124162048 main.cpp:115] Connecting to server at (1)@10.0.0.224:53492... Thanks, Niklas Nielsen
Re: Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Set-JAVA_HOME #2371
Hmm - don't know what happened here. We didn't touch libcurl - do you guys have any insight here? Niklas On 28 August 2014 15:19, Apache Jenkins Server jenk...@builds.apache.org wrote: See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Set-JAVA_HOME/2371/changes Changes: [niklas] Modifed execute cli to run docker images. -- [...truncated 260 lines...] 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/strings_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/subcommand_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/thread_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/uuid_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:179: warning: source file '$(STOUT)/tests/proc_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:179: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:179: warning: source file '$(STOUT)/tests/os/setns_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:179: but option 'subdir-objects' is disabled 3rdparty/Makefile.am: installing './depcomp' Makefile.am:32: warning: source file 'src/http.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/latch.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/metrics/metrics.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/pid.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/process.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/reap.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/subprocess.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/timeseries.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/decoder_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/encoder_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/http_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/io_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/main.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/mutex_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/metrics_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/owned_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/process_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/queue_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/reap_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/sequence_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/shared_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/statistics_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/subprocess_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/system_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file
Re: Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Set-JAVA_HOME #2371
Thanks for the prompt response Vinod! Niklas On 28 August 2014 15:28, Vinod Kone vinodk...@gmail.com wrote: libcurl is not installed on some jenkins slaves. i pinged jake farrell on a separate thread and waiting to hear back. On Thu, Aug 28, 2014 at 3:26 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hmm - don't know what happened here. We didn't touch libcurl - do you guys have any insight here? Niklas On 28 August 2014 15:19, Apache Jenkins Server jenk...@builds.apache.org wrote: See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Set-JAVA_HOME/2371/changes Changes: [niklas] Modifed execute cli to run docker images. -- [...truncated 260 lines...] 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/strings_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/subcommand_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/thread_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:145: warning: source file '$(STOUT)/tests/uuid_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:145: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:179: warning: source file '$(STOUT)/tests/proc_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:179: but option 'subdir-objects' is disabled 3rdparty/Makefile.am:179: warning: source file '$(STOUT)/tests/os/setns_tests.cpp' is in a subdirectory, 3rdparty/Makefile.am:179: but option 'subdir-objects' is disabled 3rdparty/Makefile.am: installing './depcomp' Makefile.am:32: warning: source file 'src/http.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/latch.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/metrics/metrics.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/pid.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/process.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/reap.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/subprocess.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:32: warning: source file 'src/timeseries.cpp' is in a subdirectory, Makefile.am:32: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/decoder_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/encoder_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/http_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/io_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/main.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/mutex_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/metrics_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/owned_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/process_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/queue_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/reap_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/sequence_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/shared_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning: source file 'src/tests/statistics_tests.cpp' is in a subdirectory, Makefile.am:90: but option 'subdir-objects' is disabled Makefile.am:90: warning
Re: Review Request 25124: Added 'net-bench' libprocess benchmarking tool.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25124/ --- (Updated Aug. 28, 2014, 10:19 p.m.) Review request for mesos, Benjamin Hindman and Ben Mahler. Changes --- Please disregard previous patch - wrong diff. Repository: mesos-git Description --- Added libprocess connection throughput stress test (basically two libprocess programs sending messsages back and forth). One end is multihomed so we can scale the number of clients. One client will issue multiple concurrent messages (messages sent before waiting for acknowledgement). The motivation is to stress test libprocess communication and get an idea on theoretical throughput / scale-properties for increase slave-fanout. Diffs (updated) - src/Makefile.am 40b9f6b src/net-bench/main.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25124/diff/ Testing --- Server: $ ./src/mesos-net-bench WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:01.605250 259977216 main.cpp:111] Running server at (1)@10.0.0.224:53492 I0827 15:48:13.617341 259977216 main.cpp:135] 14628.2 requests / second I0827 15:48:14.617578 258367488 main.cpp:135] 13886.7 requests / second I0827 15:48:15.617683 257830912 main.cpp:135] 8677.19 requests / second I0827 15:48:16.617771 257830912 main.cpp:135] 8304.19 requests / second Client: $ ./build/src/mesos-net-bench --server=(1)@10.0.0.224:53492 WARNING: Logging before InitGoogleLogging() is written to STDERR I0827 15:48:12.624887 124162048 main.cpp:115] Connecting to server at (1)@10.0.0.224:53492... Thanks, Niklas Nielsen
Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review52726 --- I think this will work (and good job on style btw!). The biggest piece that I find missing is testing; we need tests to verify that the new escalation logic works as we expect. Tests in case of long (more than 3 deltas) and short (smaller than delta) timeouts and combinations of the different levels responding or not-responding to SIGTERM and so on. Also, we should make sure that there are no surprises when setting the grace period. I found it a bit surprising that small timeouts gets chopped in halfs and thirds. What happens for 0 second timeouts? Would it make sense to introduce a helper class that centralize the timeout logic/computation? We can add more verification there too and can be used to test the logic directly from our unit tests. Does this make sense? src/exec/exec.cpp https://reviews.apache.org/r/25434/#comment91705 Small nit: We try to keep the variable names short and concise. I would have dropped the 'mesos' prefix. src/launcher/executor.cpp https://reviews.apache.org/r/25434/#comment91708 Till raised an issue with namespace aliasing - did you guys sort that out? I am not a fan either. src/slave/constants.hpp https://reviews.apache.org/r/25434/#comment91707 Small bit: max columns for comments are 70: http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ src/tests/containerizer.cpp https://reviews.apache.org/r/25434/#comment91706 Why 3 seconds? - Niklas Nielsen On Sept. 9, 2014, 5:54 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Sept. 9, 2014, 5:54 a.m.) Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy St. Clair. Bugs: MESOS-1571 https://issues.apache.org/jira/browse/MESOS-1571 Repository: mesos-git Description --- The configurable slave's executor_shutdown_grace_period flag is propagated to Executor and CommandExecutor through an environment variable. Shutdown timeout in Executor and signal escalation timeout in CommandExecutor are now dependent on this flag. Each nested timeout is somewhat shorter than the parent one. Diffs - src/exec/exec.cpp 36d1778 src/launcher/executor.cpp 12ac14b src/slave/constants.hpp 9030871 src/slave/constants.cpp e1da5c0 src/slave/containerizer/containerizer.hpp 8a66412 src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/docker.cpp 0febbac src/slave/containerizer/external_containerizer.cpp efbc68f src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/slave/flags.hpp 21e0021 src/tests/containerizer.cpp a17e1e0 Diff: https://reviews.apache.org/r/25434/diff/ Testing --- make check (OS X 10.9.4; Ubuntu 14.04 amd64) Thanks, Alexander Rukletsov
Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.
On Sept. 9, 2014, 9:21 a.m., Timothy Chen wrote: src/launcher/executor.cpp, line 664 https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line664 I'm not sure we've all agreed to start using C++11 yet? Good catch - no auto's yet. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review52729 --- On Sept. 9, 2014, 5:54 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Sept. 9, 2014, 5:54 a.m.) Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy St. Clair. Bugs: MESOS-1571 https://issues.apache.org/jira/browse/MESOS-1571 Repository: mesos-git Description --- The configurable slave's executor_shutdown_grace_period flag is propagated to Executor and CommandExecutor through an environment variable. Shutdown timeout in Executor and signal escalation timeout in CommandExecutor are now dependent on this flag. Each nested timeout is somewhat shorter than the parent one. Diffs - src/exec/exec.cpp 36d1778 src/launcher/executor.cpp 12ac14b src/slave/constants.hpp 9030871 src/slave/constants.cpp e1da5c0 src/slave/containerizer/containerizer.hpp 8a66412 src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/docker.cpp 0febbac src/slave/containerizer/external_containerizer.cpp efbc68f src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/slave/flags.hpp 21e0021 src/tests/containerizer.cpp a17e1e0 Diff: https://reviews.apache.org/r/25434/diff/ Testing --- make check (OS X 10.9.4; Ubuntu 14.04 amd64) Thanks, Alexander Rukletsov
Re: Review Request 23708: Fix line comments end punctuation in stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23708/#review52820 --- Ship it! Ship It! - Niklas Nielsen On Aug. 19, 2014, 5:36 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23708/ --- (Updated Aug. 19, 2014, 5:36 p.m.) Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/23708 Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 112dcb1c51315e04267edba2e1e2c86212aeecd6 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 43b1bde53bc151344087d6b5e750c13a5b8d246d 3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp 88b57979a8bd37c52719b8183aa1f8c5290fcd7c Diff: https://reviews.apache.org/r/23708/diff/ Testing --- make check Thanks, Timothy Chen
Completed tasks remains in TASK_RUNNING when framework is disconnected
Hi guys, We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. Here is a test framework we have been able to reproduce the issue with: https://gist.github.com/nqn/9b9b1de9123a6e836f54 It launches many short-lived tasks (1 second sleep) and when killing the framework instance, the master reports the tasks as running even after several minutes: http://cl.ly/image/2R3719461e0t/Screen%20Shot%202014-09-10%20at%203.19.39%20PM.png When clicking on one of the slaves where, for example, task 49 runs; the slave knows that it completed: http://cl.ly/image/2P410L3m1O1N/Screen%20Shot%202014-09-10%20at%203.21.29%20PM.png The tasks only finish when the framework connects again (which it may never do). This is on Mesos 0.20.0, but also applies to HEAD (as of today). Do you guys have any insights into what may be going on here? Is this by-design or a bug? Thanks, Niklas
Re: Completed tasks remains in TASK_RUNNING when framework is disconnected
Here is the log of a mesos-local instance where I reproduced it: https://gist.github.com/nqn/f7ee20601199d70787c0 (Here task 10 to 19 are stuck in running state). There is a lot of output, so here is a filtered log for task 10: https://gist.github.com/nqn/a53e5ea05c5e41cd5a7d At first glance, it looks like the task can't be found when trying to forward the finish update because the running update never got acknowledged before the framework disconnected. I may be missing something here. Niklas On 10 September 2014 16:09, Niklas Nielsen nik...@mesosphere.io wrote: Hi guys, We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. Here is a test framework we have been able to reproduce the issue with: https://gist.github.com/nqn/9b9b1de9123a6e836f54 It launches many short-lived tasks (1 second sleep) and when killing the framework instance, the master reports the tasks as running even after several minutes: http://cl.ly/image/2R3719461e0t/Screen%20Shot%202014-09-10%20at%203.19.39%20PM.png When clicking on one of the slaves where, for example, task 49 runs; the slave knows that it completed: http://cl.ly/image/2P410L3m1O1N/Screen%20Shot%202014-09-10%20at%203.21.29%20PM.png The tasks only finish when the framework connects again (which it may never do). This is on Mesos 0.20.0, but also applies to HEAD (as of today). Do you guys have any insights into what may be going on here? Is this by-design or a bug? Thanks, Niklas
Review Request 25588: Fixed flaky MasterTest.LaunchDuplicateOfferTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25588/ --- Review request for mesos and Jiang Yan Xu. Bugs: mesos-1783 https://issues.apache.org/jira/browse/mesos-1783 Repository: mesos-git Description --- A couple of races could occur in the launch tasks on multiple offers tests where recovered resources from purposely-failed invocations turned into a subsequent resource offer and oversaturated the expect's. Diffs - src/tests/master_tests.cpp 3d080b2 Diff: https://reviews.apache.org/r/25588/diff/ Testing --- make check - however, I haven't been able to provoke the actual fault but verified that the subsequent offers could occur (by hand). Thanks, Niklas Nielsen
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53196 --- Ship it! This is awesome! I will commit shortly: I fixed a few minor style nits (removing a duplicate comment and adding newlines). - Niklas Nielsen On Aug. 28, 2014, 9:24 a.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 28, 2014, 9:24 a.m.) Review request for mesos, Adam B and Benjamin Hindman. Bugs: MESOS-1734 https://issues.apache.org/jira/browse/MESOS-1734 Repository: mesos-git Description --- Reduce compile time: - replacing a macro expansion with a variadic template - moving implementation from help.hpp to help.cpp Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/include/process/help.hpp 4333b5b 3rdparty/libprocess/src/help.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25079/diff/ Testing --- Ran make check. Thanks, Patrick Reilly
Re: 0.20.1 Release Manager
How about Till? I can help out too. Niklas On Friday, September 12, 2014, Tim St Clair tstcl...@redhat.com wrote: I would like to, but this next month is going to be crazy for me. Perhaps the next cycle. Regards, Tim - Original Message - From: Vinod Kone vinodk...@gmail.com javascript:; To: dev dev@mesos.apache.org javascript:; Sent: Friday, September 12, 2014 12:22:59 PM Subject: Re: 0.20.1 Release Manager Tim St. Clair or Adam, would you guys be willing to work with Bhuvan to get the release out? On Fri, Sep 12, 2014 at 10:12 AM, Benjamin Mahler benjamin.mah...@gmail.com javascript:; wrote: Hi Bhuvan, that's great! However, doing a release requires commit access. On Sep 12, 2014, at 8:06 AM, Bhuvan Arumugam bhu...@apache.org javascript:; wrote: I like to volunteer for this role. There are 18 bugs targetted for this release. If we want to target any other bug fixes for this release, please ensure Target version flag is set as 0.20.1 for that bug. https://issues.apache.org/jira/browse/MESOS-1621?jql=project%20%3D%20MESOS%20AND%20%22Target%20Version%2Fs%22%20%3D%200.20.1 15 out of 18 are either resolved/merged or patch in reviewboard. The following 3 bugs are not in progress. If we dont intend to fix these bugs in this release, please remove 0.20.1 from Target version. https://issues.apache.org/jira/browse/MESOS-1776 - --without-PACKAGE will set incorrect dependency prefix https://issues.apache.org/jira/browse/MESOS-1775 - Libprocess wants source for unbundled gmock https://issues.apache.org/jira/browse/MESOS-1741 - mesos-slave shouldn't fail if dockerd is down I'll touch base on IRC when we will be ready to start the voting. /me is bhuvan on IRC On Thu, Sep 11, 2014 at 10:41 PM, Jie Yu yujie@gmail.com javascript:; wrote: I am a little swamped recently, anyone volunteer for the release manager? - Jie On Thu, Sep 11, 2014 at 6:04 PM, Benjamin Mahler benjamin.mah...@gmail.com javascript:; wrote: Do we have a volunteer to be the 0.20.1 release manager? Looks like a number of tickets are being added at this point for 0.20.1: https://issues.apache.org/jira/issues/?jql=project%20%3D%20MESOS%20AND%20%22Target%20Version%2Fs%22%20%3D%200.20.1 -- Regards, Bhuvan Arumugam www.livecipher.com -- Cheers, Timothy St. Clair Red Hat Inc. -- Niklas
Re: Review Request 22526: WIP:Added resizeTask primitive.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22526/#review53230 --- I don't think Yifan is going to complete this work. I am going to discard for now if you don't have any objections. - Niklas Nielsen On June 17, 2014, 11:23 a.m., Yifan Gu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22526/ --- (Updated June 17, 2014, 11:23 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-1279 https://issues.apache.org/jira/browse/MESOS-1279 Repository: mesos-git Description --- Added resizeTask primitive. This is just a proof of concept now. I will work on the unit test. Currently I added one state called TASK_RESIZE in state update, so that the master/framework can get the resize result from the slave. I put the result in the 'data' field of the TaskStatus. And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later. Any question or suggestion is highly welcome! Thanks! Diffs - include/mesos/mesos.proto 102289b include/mesos/scheduler.hpp d224945 include/mesos/scheduler/scheduler.proto 6ab5089 src/Makefile.am c91b438 src/common/protobuf_utils.hpp 12ff00a src/master/master.hpp 7a12185 src/master/master.cpp 4a01b1a src/messages/messages.proto 8aecc8b src/sched/sched.cpp 6e14f1c src/scheduler/scheduler.cpp 4ae188e src/slave/slave.hpp 34687e5 src/slave/slave.cpp 643c088 src/tests/resize_task_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/22526/diff/ Testing --- make check. Thanks, Yifan Gu
Re: Review Request 25042: Remove duplicate headers imports
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25042/#review53231 --- Ship it! Cool! Going to commit this in a bit - Niklas Nielsen On Aug. 25, 2014, 3:57 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25042/ --- (Updated Aug. 25, 2014, 3:57 p.m.) Review request for mesos, Adam B and Niklas Nielsen. Repository: mesos-git Description --- Remove duplicate headers imports Diffs - src/java/jni/org_apache_mesos_Log.cpp 05489135fbd890073c58acf55312d621c7a790e7 src/sched/sched.cpp 447b96fd88111536fdd7704abd078cc16b5c7943 src/scheduler/scheduler.cpp b7ce4de4b01a898bb6763dde070fe521e142ee6c src/tests/zookeeper.cpp 29656eb423eaf2e72e5323eca66fd757dc7d2a37 Diff: https://reviews.apache.org/r/25042/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25042: Remove duplicate headers imports
On Sept. 12, 2014, 3:07 p.m., Niklas Nielsen wrote: Cool! Going to commit this in a bit Well, never mind - Adam already got this through. Remember to mark as submitted :) - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25042/#review53231 --- On Aug. 25, 2014, 3:57 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25042/ --- (Updated Aug. 25, 2014, 3:57 p.m.) Review request for mesos, Adam B and Niklas Nielsen. Repository: mesos-git Description --- Remove duplicate headers imports Diffs - src/java/jni/org_apache_mesos_Log.cpp 05489135fbd890073c58acf55312d621c7a790e7 src/sched/sched.cpp 447b96fd88111536fdd7704abd078cc16b5c7943 src/scheduler/scheduler.cpp b7ce4de4b01a898bb6763dde070fe521e142ee6c src/tests/zookeeper.cpp 29656eb423eaf2e72e5323eca66fd757dc7d2a37 Diff: https://reviews.apache.org/r/25042/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25597: Added a version checker class to stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53352 --- Ship it! Looks like it is ready to go :) 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp https://reviews.apache.org/r/25597/#comment92982 Review bot caught this; would you mind moving the class body 2 spaces left? We don't indent public, private, etc - Niklas Nielsen On Sept. 12, 2014, 5:14 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/ --- (Updated Sept. 12, 2014, 5:14 p.m.) Review request for mesos, Adam B and Niklas Nielsen. Repository: mesos-git Description --- Currently there is no facility in Mesos for checking compatibility of various Mesos components that could have been built at different times with potentially different Mesos versions. This requirement is especially important for doing various compatibility checks between Mesos and Mesos modules (WIP). - Features major, minor, and patch numbers. - Convenience functions for comparing two versions. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am db9766d70adb9076946cd2b467c55636fe5f7235 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de53c3873ecd0b62a08ca9aac530043ffb9 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6fa5b741bdd7f089ba93bf6fea43b9f39f8f0edb 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25597/diff/ Testing --- Added a stout test and ran make check Thanks, Kapil Arya
Re: Review Request 25614: Safer handling of futures in JNI state abstraction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53408 --- Ship it! Ship It! - Niklas Nielsen On Sept. 15, 2014, 1:54 p.m., Connor Doyle wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/ --- (Updated Sept. 15, 2014, 1:54 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-1795 https://issues.apache.org/jira/browse/MESOS-1795 Repository: mesos-git Description --- ## OVERVIEW This change adds a measure of safety for calls to `FutureT::await()` without a timeout from within the JNI interface to the state abstraction. ## MOTIVATION Observed the following log output prior to a crash of the Marathon scheduler: ``` Sep 12 23:46:01 highly-available-457-540 marathon[11494]: F0912 23:46:01.771927 11532 org_apache_mesos_state_AbstractState.cpp:145] CHECK_READY(*future): is PENDING Sep 12 23:46:01 highly-available-457-540 marathon[11494]: *** Check failure stack trace: *** Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663a2d google::LogMessage::Fail() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26657e3 google::LogMessage::SendToLog() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663648 google::LogMessage::Flush() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc266603e google::LogMessageFatal::~LogMessageFatal() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26588a3 Java_org_apache_mesos_state_AbstractState__1_1fetch_1get Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febcd107d98 (unknown) ``` _Listing 1: Crash log output._ ## CHANGES IN DETAIL The in-line comments in `Latch::await(const Duration duration)`, mention that it's possible for the call to return before the underlying process has completed for two reasons: ``` ... process::wait(pid, duration); // Explict to disambiguate. // It's possible that we failed to wait because: // (1) Our process has already terminated. // (2) We timed out (i.e., duration was not infinite). ... ``` _Listing 2: Excerpt from 3rdparty/src/libprocess/latch.cpp._ Call sites within `org_apache_mesos_state_AbstractState.cpp` that formerly discarded the return value now throw a `java.concurrent.ExecutionException` if the await fails. I chose to throw this instead of a `java.concurrent.TimeoutException` since the expectation from the caller's perspective is to block forever pending a ready state. Diffs - src/java/jni/org_apache_mesos_state_AbstractState.cpp 1accc8a Diff: https://reviews.apache.org/r/25614/diff/ Testing --- - make - make check Thanks, Connor Doyle
Re: Review Request 25121: Try to give a hint as to what might have gone wrong
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25121/#review53410 --- Can you add some context to the background of this RR? - Niklas Nielsen On Aug. 27, 2014, 2:07 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25121/ --- (Updated Aug. 27, 2014, 2:07 p.m.) Review request for mesos, Adam B and Benjamin Hindman. Bugs: MESOS-1735 https://issues.apache.org/jira/browse/MESOS-1735 Repository: mesos-git Description --- Output Already running Diffs - 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25121/diff/ Testing --- This is currently a work in progress, (WIP) Thanks, Patrick Reilly
Re: Review Request 25614: Safer handling of futures in JNI state abstraction
On Sept. 15, 2014, 2:17 p.m., Jie Yu wrote: This more looks like a bug somewhere. Let's try to find out the true root cause first. True - but throwing the result away from await() seems silly/unsafe too (rather on relying on the await to have completed or wait forever). Our investigation ended in https://github.com/apache/mesos/blob/master/3rdparty/libprocess/src/latch.cpp#L58 (as explained in the RR description). We haven't been able to realiably reproduce the fault to debug it - Connor, can you fill in on when it happened in Marathon? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53409 --- On Sept. 15, 2014, 1:54 p.m., Connor Doyle wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/ --- (Updated Sept. 15, 2014, 1:54 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-1795 https://issues.apache.org/jira/browse/MESOS-1795 Repository: mesos-git Description --- ## OVERVIEW This change adds a measure of safety for calls to `FutureT::await()` without a timeout from within the JNI interface to the state abstraction. ## MOTIVATION Observed the following log output prior to a crash of the Marathon scheduler: ``` Sep 12 23:46:01 highly-available-457-540 marathon[11494]: F0912 23:46:01.771927 11532 org_apache_mesos_state_AbstractState.cpp:145] CHECK_READY(*future): is PENDING Sep 12 23:46:01 highly-available-457-540 marathon[11494]: *** Check failure stack trace: *** Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663a2d google::LogMessage::Fail() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26657e3 google::LogMessage::SendToLog() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663648 google::LogMessage::Flush() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc266603e google::LogMessageFatal::~LogMessageFatal() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26588a3 Java_org_apache_mesos_state_AbstractState__1_1fetch_1get Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febcd107d98 (unknown) ``` _Listing 1: Crash log output._ ## CHANGES IN DETAIL The in-line comments in `Latch::await(const Duration duration)`, mention that it's possible for the call to return before the underlying process has completed for two reasons: ``` ... process::wait(pid, duration); // Explict to disambiguate. // It's possible that we failed to wait because: // (1) Our process has already terminated. // (2) We timed out (i.e., duration was not infinite). ... ``` _Listing 2: Excerpt from 3rdparty/src/libprocess/latch.cpp._ Call sites within `org_apache_mesos_state_AbstractState.cpp` that formerly discarded the return value now throw a `java.concurrent.ExecutionException` if the await fails. I chose to throw this instead of a `java.concurrent.TimeoutException` since the expectation from the caller's perspective is to block forever pending a ready state. Diffs - src/java/jni/org_apache_mesos_state_AbstractState.cpp 1accc8a Diff: https://reviews.apache.org/r/25614/diff/ Testing --- - make - make check Thanks, Connor Doyle
Re: Completed tasks remains in TASK_RUNNING when framework is disconnected
? On Wed, Sep 10, 2014 at 5:55 PM, Vinod Kone vinodk...@gmail.com wrote: The main reason is to keep status update manager simple. Also, it is very easy to enforce the order of updates to the master/framework in this model. If we allow multiple updates for a task to be in flight, it's really hard (impossible?) to ensure that we are not delivering out-of-order updates even in edge cases (failover, network partitions etc). On Wed, Sep 10, 2014 at 5:35 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hey Vinod - thanks for chiming in! Is there a particular reason for only having one status in flight? Or to put it in another way, isn't that too strict behavior taken that the master state could present the most recent known state if the status update manager tried to send more than the front of the stream? Taken very long timeouts, just waiting for those to disappear seems a bit tedious and hogs the cluster. Niklas On 10 September 2014 17:18, Vinod Kone vinodk...@gmail.com wrote: What you observed is expected because of the way the slave (specifically, the status update manager) operates. The status update manager only sends the next update for a task if a previous update (if it exists) has been acked. In your case, since TASK_RUNNING was not acked by the framework, master doesn't know about the TASK_FINISHED update that is queued up by the status update manager. If the framework never comes back, i.e., failover timeout elapses, master shuts down the framework, which releases those resources. On Wed, Sep 10, 2014 at 4:43 PM, Niklas Nielsen nik...@mesosphere.io wrote: Here is the log of a mesos-local instance where I reproduced it: https://gist.github.com/nqn/f7ee20601199d70787c0 (Here task 10 to 19 are stuck in running state). There is a lot of output, so here is a filtered log for task 10: https://gist.github.com/nqn/a53e5ea05c5e41cd5a7d At first glance, it looks like the task can't be found when trying to forward the finish update because the running update never got acknowledged before the framework disconnected. I may be missing something here. Niklas On 10 September 2014 16:09, Niklas Nielsen nik...@mesosphere.io wrote: Hi guys, We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. Here is a test framework we have been able to reproduce the issue with: https://gist.github.com/nqn/9b9b1de9123a6e836f54 It launches many short-lived tasks (1 second sleep) and when killing the framework instance, the master reports the tasks as running even after several minutes: http://cl.ly/image/2R3719461e0t/Screen%20Shot%202014-09-10%20at%203.19.39%20PM.png When clicking on one of the slaves where, for example, task 49 runs; the slave knows that it completed: http://cl.ly/image/2P410L3m1O1N/Screen%20Shot%202014-09-10%20at%203.21.29%20PM.png The tasks only finish when the framework connects again (which it may never do). This is on Mesos 0.20.0, but also applies to HEAD (as of today). Do you guys have any insights into what may be going on here? Is this by-design or a bug? Thanks, Niklas
Re: Completed tasks remains in TASK_RUNNING when framework is disconnected
Okay - that only solves half of the problem for us: users will still see their frameworks as running even though they completed but it is a first step. Let's continue the discussion in a JIRA ticket; I'll create one shortly. Thanks for helping out! Niklas On 15 September 2014 18:17, Benjamin Mahler benjamin.mah...@gmail.com wrote: On Mon, Sep 15, 2014 at 3:11 PM, Niklas Nielsen nik...@mesosphere.io wrote: Thanks for your input Ben! (Comments inlined) On 15 September 2014 12:35, Benjamin Mahler benjamin.mah...@gmail.com wrote: To ensure that the architecture of mesos remains a scalable one, we want to persist state in the leaves of the system as much as possible. This is why the master has never persisted tasks, task states, or status updates. Note that status updates can contain arbitrarily large amounts of data at the current time (example impact of this: https://issues.apache.org/jira/browse/MESOS-1746). Adam, I think solution (2) you listed above of including a terminal indicator in the _private_ message between slave and master would easily allow us to release the resources at the correct time. We would still hold the correct task state in the master, and would maintain the status update stream invariants for frameworks (guaranteed ordered delivery). This would be simpler to implement with my recent change here, because you no longer have to remove the task to release the resources: https://reviews.apache.org/r/25568/ We should still hold the correct task state; meaning the actual state of the task on the slave? Correct, as in, just maintaining the existing task state semantics in the master (for correctness of reconciliation / status update ordering). Then the auxiliary field should represent last known status, which may not necessarily be terminal. For example, a staging update followed by a running update while the framework is disconnected will show as staging still - or am I missing something? It would still show as staging, the running update would never be sent to the master since the slave is not receiving acknowledgements. But if there were a terminal task, then the slave would be setting the auxiliary field in the StatusUpdateMessage and the master will know to release the resources. + backwards compatibility Longer term, adding pipelining of status updates would be a nice improvement (similar to what you listed in (1) above). But as Vinod said, it will require care to ensure we maintain the stream invariants for frameworks (i.e. probably need to send multiple updates in 1 message). How does this sound? Sounds great - we would love to help out with (2). Would you be up for shepherding such a change? Yep! The changes needed should be based off of https://reviews.apache.org/r/25568/ since it changes the resource releasing in the master. On Thu, Sep 11, 2014 at 12:02 PM, Adam Bordelon a...@mesosphere.io wrote: Definitely relevant. If the master could be trusted to persist all the task status updates, then they could be queued up at the master instead of the slave once the master has acknowledged its receipt. Then the master could have the most up-to-date task state and can recover the resources as soon as it receives a terminal update, even if the framework is disconnected and unable to receive/ack the status updates. Then, once the framework reconnects, the master will be responsible for sending its queued status updates. We will still need a queue on the slave side, but only for updates that the master has not persisted and ack'ed, primarily during the scenario when the slave is disconnected from the master. On Thu, Sep 11, 2014 at 10:17 AM, Vinod Kone vinodk...@gmail.com wrote: The semantics of these changes would have an impact on the upcoming task reconciliation. @BenM: Can you chime in here on how this fits into the task reconciliation work that you've been leading? On Wed, Sep 10, 2014 at 6:12 PM, Adam Bordelon a...@mesosphere.io wrote: I agree with Niklas that if the executor has sent a terminal status update to the slave, then the task is done and the master should be able to recover those resources. Only sending the oldest status update to the master, especially in the case of framework failover, prevents these resources from being recovered in a timely manner. I see a couple of options for getting around this, each with their own disadvantages. 1) Send the entire status update stream to the master. Once the master sees the terminal status update, it will removeTask and recover the resources. Future resends of the update will be forwarded to the scheduler, but the master will ignore (with warning and invalid_update
Re: Review Request 25785: Add guide to becoming a committer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/#review53843 --- Where do these dead-lines come from? Has there been any agreement on this process in general and doesn't this deserve to be discussed and consensus reached (dev@ or private) before putting out a document on it? I might be missing something here of course; if so, can you point me to the references? - Niklas Nielsen On Sept. 18, 2014, 10:41 a.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/ --- (Updated Sept. 18, 2014, 10:41 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-1815 https://issues.apache.org/jira/browse/MESOS-1815 Repository: mesos-git Description --- Add a guide to becoming a committer. Diffs - docs/becoming-a-committer.md PRE-CREATION Diff: https://reviews.apache.org/r/25785/diff/ Testing --- Thanks, Dominic Hamon
Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25798/#review53889 --- 3rdparty/libprocess/include/process/help.hpp https://reviews.apache.org/r/25798/#comment93745 sticks to T, so 'T' (consistent with current style - know that we have been discussing how it actually binds). Here and below. 3rdparty/libprocess/include/process/help.hpp https://reviews.apache.org/r/25798/#comment93746 Can we rename 'ss' to something more descriptive? Here and below. 3rdparty/libprocess/include/process/help.hpp https://reviews.apache.org/r/25798/#comment93747 std::endl? 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93748 Unused? 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93752 'using std::string' and trim rest of file? :-) 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93741 Is this a copy of the help text in the header? If so, let's delete it here. 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93743 Two new lines between implementing functions here and rest of file. 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93749 Mind indenting per http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93751 The indentation in the return here (and below) seems to be off. 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25798/#comment93750 The indentation seem a bit off here. - Niklas Nielsen On Sept. 18, 2014, 3:53 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25798/ --- (Updated Sept. 18, 2014, 3:53 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Follow up from 25079. Reduces compile time overall by ~1/3rd. Move implementation of libprocess help into cpp file. Use strings::join to refactor macro expansion that was injecting lots of overhead even when the expanded functions were not used. Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/include/process/help.hpp 4333b5b 3rdparty/libprocess/src/help.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25798/diff/ Testing --- make check on 3rdparty support/mesos-style.py Thanks, Joris Van Remoortere
Re: [VOTE] Release Apache Mesos 0.20.1 (rc3)
+1 (binding) On Friday, September 19, 2014, Till Toenshoff toensh...@me.com wrote: +1 make check on OSX 10.9.5, clang 3.5 (Xcode 6) -all tests passed On Sep 19, 2014, at 3:03 AM, Vinod Kone vinodk...@gmail.com javascript:_e(%7B%7D,'cvml','vinodk...@gmail.com'); wrote: +1 (binding) sudo make check on CentOS 5.5 w/ gcc-4.8.2 On Thu, Sep 18, 2014 at 4:05 PM, Adam Bordelon a...@mesosphere.io javascript:_e(%7B%7D,'cvml','a...@mesosphere.io'); wrote: Hi all, Please vote on releasing the following candidate as Apache Mesos 0.20.1. 0.20.1 includes the following: Minor bug fixes for docker integration, network isolation, build, etc. The CHANGELOG for the release is available at: https://git-wip-us.apache.org/repos/asf?p=mesos.git;a=blob_plain;f=CHANGELOG;hb=0.20.1-rc3 The candidate for Mesos 0.20.1 release is available at: https://dist.apache.org/repos/dist/dev/mesos/0.20.1-rc3/mesos-0.20.1.tar.gz The tag to be voted on is 0.20.1-rc3: https://git-wip-us.apache.org/repos/asf?p=mesos.git;a=commit;h=0.20.1-rc3 The MD5 checksum of the tarball can be found at: https://dist.apache.org/repos/dist/dev/mesos/0.20.1-rc3/mesos-0.20.1.tar.gz.md5 The signature of the tarball can be found at: https://dist.apache.org/repos/dist/dev/mesos/0.20.1-rc3/mesos-0.20.1.tar.gz.asc The PGP key used to sign the release is here: https://dist.apache.org/repos/dist/release/mesos/KEYS The JAR is up in Maven in a staging repository here: https://repository.apache.org/content/repositories/orgapachemesos-1036 Please vote on releasing this package as Apache Mesos 0.20.1! The vote is open until Mon Sep 22 17:00:00 PDT 2014 and passes if a majority of at least 3 +1 PMC votes are cast. [ ] +1 Release this package as Apache Mesos 0.20.1 [ ] -1 Do not release this package because ... Thanks, Adam and Bhuvan
Mesos Modules Design
Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and https://issues.apache.org/jira/browse/MESOS-1384. Kapil and Bernd took over the work (shepherded by Ben H and I) and have expanded on the original design to cope with api/modules/mesos versioning semantics and be extensible enough to cope future changes in the modules subsystem (dealing with modules dependencies, etc). The latest description of the modules system has been captured in: https://cwiki.apache.org/confluence/display/MESOS/DRAFT+Design+Doc+-+Mesos+Modules (for those of you who don't want to read through the JIRA threads). We have an implementation ready based on this design and will be sharing / starting review rounds start next week. Feel free to use this thread if you have any questions. Cheers, Niklas
Re: Review Request 25847: Refactor Libprocess: class Node
On Sept. 19, 2014, 1:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 32 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line32 uint64_t? start getting ipv6 support in... ;) Nikita Vetoshkin wrote: Still won't fit in ipv6 though :( Joris Van Remoortere wrote: We can use code__uint128_t/code ; however, I am not sure if this is supported accross all our required compilers. The larger issue is that the uint32_t type is currently baked into the protobufs, so this would be a larger compatibility change. I would suggest to post-pone the ipv6 concerns to a full patch which supports it (and tests it). On Sept. 19, 2014, 1:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 11 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line11 worth renaming to IpPort? or Host? With several concepts (including this) being inspired by Erlang conventions, I wouldn't change 'Node': http://www.erlang.org/doc/reference_manual/distributed.html - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54018 --- On Sept. 19, 2014, 1:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 1:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54030 --- Ship it! Can you motivate it a bit in context of eventually reworking the libprocess transport/event: https://issues.apache.org/jira/browse/MESOS-1330 and down-sizing the 4000+ line process.cpp :-) 3rdparty/libprocess/include/process/node.hpp https://reviews.apache.org/r/25847/#comment93927 Know it is not yours, but these fit on a single line. Thanks! :) We try to use postfix underscore now to avoid clashes, https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html I will leave that up to you whether you want to address it here. - Niklas Nielsen On Sept. 19, 2014, 1:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 1:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Mesos Modules Design
Hi Dominic, (response inlined) On 19 September 2014 13:03, Dominic Hamon dha...@twopensource.com wrote: I'm sorry, but I'm still having a hard time understanding why this needs to be dynamic. If the mesos core is split into modules that are built as standalone libraries (static) then at link time the right combination of libraries can be bundled together to create the end result. If you want to get even smarter, we can have default versions that are linked in to the mesos core as weak symbols so later linked libraries can override the defaults. This may mean that we move to static linking across the board, but frankly there are a few benefits to that approach. This works for some, but not all use cases. One use-case where it _does_ make sense to statically bake into the image could be: https://issues.apache.org/jira/browse/MESOS-1330 That be, you probably rarely want to swap network transport implementations in and out on per-run basis but will know up front which one to configure and use. On the other hand, having to relink and rebuilding give users a poor experience and makes it hard to select (or unselect) custom components. Using a prebuilt package and point against libraries is a pretty common work-flow: Apache web server relies heavily on modules from dynamic libraries. TL;DR I don't see the two approaches to be mutually exclusive and we can get a lot of leverage with the current design but we want to be able to do this with static linking too. (To Tim St Clair's point) With the approach as defined, does this mean that the default versions will also have to be reimplemented as modules? Not necessary. Mesos default implementations stays as is. See Bernd's comment. Has any effort been put into determining the performance overhead of the approach as specified? It won't affect Mesos if you are not using modules. The call sites will be virtual dispatches no matter whether you are using modules or internal/default implementations. There is a performance penalty of not being able to do global optimizations within the module, but that is the trade-off of implementing a dynamic loadable module. On Fri, Sep 19, 2014 at 11:35 AM, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and https://issues.apache.org/jira/browse/MESOS-1384. Kapil and Bernd took over the work (shepherded by Ben H and I) and have expanded on the original design to cope with api/modules/mesos versioning semantics and be extensible enough to cope future changes in the modules subsystem (dealing with modules dependencies, etc). The latest description of the modules system has been captured in: https://cwiki.apache.org/confluence/display/MESOS/DRAFT+Design+Doc+-+Mesos+Modules (for those of you who don't want to read through the JIRA threads). We have an implementation ready based on this design and will be sharing / starting review rounds start next week. Feel free to use this thread if you have any questions. Cheers, Niklas -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.* Hope that helps; if not, we'd be happy to jump on a call. Niklas
Re: Review Request 25848: Introducing mesos modules.
/#comment93951 Mind throwing in a comment on why we expect exactly those libraries? src/tests/module_tests.cpp https://reviews.apache.org/r/25848/#comment93953 Two new lines src/tests/module_tests.cpp https://reviews.apache.org/r/25848/#comment93952 Where are we going to capture the negative tests? Wrong library versions, trying to load non-mesos modules libraries (libc.so for example)? - Niklas Nielsen On Sept. 19, 2014, 1:40 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 19, 2014, 1:40 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 src/tests/module.hpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Mesos Modules Design
Agreed - I can do early morning PST any day during this week (for people dialing in from the east coast and Europe). Niklas On 22 September 2014 07:15, Tim St Clair tstcl...@redhat.com wrote: Folks, I think this thread has fallen off the rails a bit, perhaps we could get a tcon/hangout going for this again. I would be happy to moderate if needed, but it's pretty clear to me to that trying to hash this out in an email thread is becoming couter-productive. Cheers, Tim - Original Message - From: Dominic Hamon dha...@twitter.com.INVALID To: dev dev@mesos.apache.org Sent: Saturday, September 20, 2014 9:21:59 AM Subject: Re: Mesos Modules Design On Fri, Sep 19, 2014 at 2:43 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Dominic, (response inlined) On 19 September 2014 13:03, Dominic Hamon dha...@twopensource.com wrote: I'm sorry, but I'm still having a hard time understanding why this needs to be dynamic. If the mesos core is split into modules that are built as standalone libraries (static) then at link time the right combination of libraries can be bundled together to create the end result. If you want to get even smarter, we can have default versions that are linked in to the mesos core as weak symbols so later linked libraries can override the defaults. This may mean that we move to static linking across the board, but frankly there are a few benefits to that approach. This works for some, but not all use cases. One use-case where it _does_ make sense to statically bake into the image could be: https://issues.apache.org/jira/browse/MESOS-1330 That be, you probably rarely want to swap network transport implementations in and out on per-run basis but will know up front which one to configure and use. On the other hand, having to relink and rebuilding give users a poor experience and makes it hard to select (or unselect) custom components. Using a prebuilt package and point against libraries is a pretty common work-flow: Apache web server relies heavily on modules from dynamic libraries. I'm going to argue against this, so please bear with me while I work through it, and please point out anywhere that my assertions are wrong. The proposal you are suggesting requires the following: - callsites need to be modules aware to use the right factory method to instantiate the modular object - mesos-core owners need to be aware of modules and versioning when they change the abstract API - module owners need to be aware of the modules API and versioning - customers (end-users) have to be modules aware and set their runtime configuration correctly - errors in versioning or modules will only be caught at runtime My alternative proposal requires the following: - module owners need to be aware of API changes during core upgrades - issues are caught at compile time by module owners - customers (end-users) are unaware of modules and only need to know about per module configuration (kerberos attributes, for instance) not the modules system and configuration In short, your proposal adds more complexity and shifts the burden of knowledge and responsibility to every agent, instead of keeping it with the module owner where it belongs. I think the solution that's been put up for review is clever, but I don't think it solves the problem in the right way for anyone involved. Please rethink your approach to take that into account. TL;DR I don't see the two approaches to be mutually exclusive and we can get a lot of leverage with the current design but we want to be able to do this with static linking too. (To Tim St Clair's point) With the approach as defined, does this mean that the default versions will also have to be reimplemented as modules? Not necessary. Mesos default implementations stays as is. See Bernd's comment. Has any effort been put into determining the performance overhead of the approach as specified? It won't affect Mesos if you are not using modules. The call sites will be virtual dispatches no matter whether you are using modules or internal/default implementations. There is a performance penalty of not being able to do global optimizations within the module, but that is the trade-off of implementing a dynamic loadable module. On Fri, Sep 19, 2014 at 11:35 AM, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and https
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 12:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 19, 2014, 5:58 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 19, 2014, 5:58 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
On Sept. 22, 2014, 5:42 p.m., Timothy Chen wrote: src/master/master.cpp, line 4401 https://reviews.apache.org/r/25911/diff/1/?file=700520#file700520line4401 Seems like resources recovered is only used internally for the master, any reason why introducing a new protobuf field instead of just storing it locally? Thanks for bringing this up - Ben M reached out on IRC and I am working on a refactor which introduce a task struct where we can hang this off. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/#review54221 --- On Sept. 22, 2014, 3:30 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- (Updated Sept. 22, 2014, 3:30 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Mesos Modules Design
(Inlined) Thanks for chiming in! On 23 September 2014 11:59, Vinod Kone vinodk...@gmail.com wrote: Ok. I finally had a chance to read the design doc, go through the comments on this thread and glance at the review. Here are my comments. I like the concept of dynamic loading of libraries when it comes to making the lives of end users/operators easy, esp when they want to mix and match the modules. I agree with Dominic though that we should make sure to minimize the burden/overhead of core developers when it comes to dealing with modules. FWIW, the implementation that I've seen seems easy enough to deal with and doesn't look like it adds much of a performance overhead. That said, the versioning part seems a bit complicated, as BenH alluded to. Can we come up with something that's simple as the first cut but still extensible? Also, it would be great if interface breakage could be automatically deduced at load time. But I guess we still need a module version to deal with protocol changes without interface changes. To ensure the devs update the versions in module manager, I suggest adding a comment on top of the current components (Allocator, Isolator etc) mentioning that there is a corresponding module that needs to be updated. Better yet, maybe we could use a naming convention (e.g., s/Allocator/AllocatorModule/) for easy visual cue. Good point on naming explicitly. We can do that incrementally when we start wiring modules up. The easiest path forward (but a bit oversimplified and non-backwards compatible) is to enforce module X to be built against the same version of mesos as the one being loaded into. It is a minor change to the current patch and per-subsystem versioning can be reenabled later on, when we gain more confidence in the API stability of individual subsystems. I am not religious there; we can start out strict. More importantly, I would like to understand how we are going to ensure the quality of modules being written. In other words how do we, as a community, ensure that a badly written module doesn't leave a bad taste in users adopting Mesos. Should we come up with a test suite (functional and performance) that runs on CI that module writers are required to test against? What is the contract for support when something in modules break? Should the users/operators ask on #mesos and dev lists or module writer's lists? Would the users even know where the problem lies? Would Mesos devs always know? I think this is where most of the burden/overhead lies. Of course, this is the same problem with frameworks, but I think this is much more relevant for modules because they (could) fundamentally change the behavior of Mesos. The first module we planned to wire up would be isolator modules. The patch is going to include a modularized isolator, tested against our existing isolator unit tests. We can work on getting the necessary build tooling setup so module implementors can run relevant unit tests (and wire up their own). I like the idea of mesos modules mailing list, as it (like with other module / plugin systems) is on the users / module writers own responsibility to be compatible, hardened and tested. The is a disclaimer that is important to emphasize. Even more fundamentally, how are we going to ensure that Mesos core gets better features vs having the cool features developed as (possibly paid) modules? For example, should Kerberos auth and SSL transport be modules or should they be integrated into the core? As an open source project, how do we ensure that the community resources are properly utilized in a fair and neutral manner to help Mesos core grow? Are we going to have guidelines/opinions on what should/could be modularized or is everything fair game? It would be great to understand how other successful open source projects toe this line. Has anyone done any research regarding this? I agree and I think we should document this in-depth. Like I mentioned above, using 3rd party modules are without _any_ guarantee or support from the Mesos developers. If we end up embracing / blessing modules, we can pull those into the source base and test it in our CI. Apache Webserver use this scheme and has graced modules ( http://httpd.apache.org/docs/current/mod/) and 3rd party / non-graced modules (https://modules.apache.org) The most commonly used (which I have been referring to): Apache Web Server. Apart from the Apache web server, the Linux loadable modules is a great example of big system that uses modules. See section 2.3 at http://www.tldp.org/HOWTO/Module-HOWTO/x73.html for the case for modules for linux. Lots of the advantages apply here as well (multiple ways to do something don't want to rebuilt the kernel all the time) As we go on and grow the capabilities of Mesos, I suspect we will need to be able to address specialized setups: custom fitted isolation mechanisms (which are going to be vastly different for cloud/VM environments to
Review Request 25967: Added task struct in master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25967/ --- Review request for mesos and Ben Mahler. Repository: mesos-git Description --- In order to hang additional state off tasks (as 'resources_recovered' in https://reviews.apache.org/r/25911/), this patch introduce a task struct to represent tasks within the master. Diffs - src/common/http.hpp afce7fe src/master/http.cpp 3f5a01d src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 Diff: https://reviews.apache.org/r/25967/diff/ Testing --- make check Thanks, Niklas Nielsen
Review Request 25986: Added reconcileTasks to python scheduler.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25986/ --- Review request for mesos and Ben Mahler. Repository: mesos-git Description --- The last step of wiring up reconcileTasks in the python bindings. Diffs - src/python/interface/src/mesos/interface/__init__.py 818f41b Diff: https://reviews.apache.org/r/25986/diff/ Testing --- Functional testing Thanks, Niklas Nielsen
Re: Review Request 25848: Introducing mesos modules.
On Sept. 24, 2014, 1:05 p.m., Timothy St. Clair wrote: I still debate whether we should name it plugin vs. module. Thoughts? Could be called either or, but think the term 'module' is pretty clear and is what we have gone with so far. I don't see any good reason to change it now unless anyone have some good references why it should be called 'plugins' (and not modules). +1 on your comments - threw in a couple of questions. On Sept. 24, 2014, 1:05 p.m., Timothy St. Clair wrote: include/mesos/module.hpp, line 73 https://reviews.apache.org/r/25848/diff/5/?file=701882#file701882line73 Perhaps we can breakout in another JIRA, but I would to denote both some form of AUTHORING as well as define api's as EXPERIMENTAL. In another JIRA to discuss the format of the macros? Or what should the outcome be of the JIRA thread? +1 on having AUTHORING. How did you think about incorporating the experimentalness of the api? in the macro name? On Sept. 24, 2014, 1:05 p.m., Timothy St. Clair wrote: src/module/manager.cpp, line 173 https://reviews.apache.org/r/25848/diff/5/?file=701887#file701887line173 You could probably stick the json parsing into a separate sub-class. I'm all for breaking out a small JIRA tree from the comments. Is it so we can discuss the format further or because we should address this in different patches? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54425 --- On Sept. 22, 2014, 9:05 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 9:05 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- (Updated Sept. 24, 2014, 3:04 p.m.) Review request for mesos and Ben Mahler. Changes --- Based on r25967 refactor Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs (updated) - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 25789: Variadic strings join
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789/#review54463 --- The last nits: looks good to me, but would like BenH to take a look :) 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94605 How about renaming ss to 'stream' or something more descriptive? (Here and below) 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94606 Same for 'val' - let's expand it to a full word. Here and below 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94604 should be 4 space indent. - Niklas Nielsen On Sept. 23, 2014, 10:30 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789/ --- (Updated Sept. 23, 2014, 10:30 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Add Variadic strings join. There is a second version of the variadic join which takes a reference to a stringstream as a parameter. This is handy when strings::join is just a part of a larger string manipulation. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25789/diff/ Testing --- Ran make check for stout. Added test cases for join as these were missing. Thanks, Joris Van Remoortere
Re: Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- (Updated Sept. 24, 2014, 4:51 p.m.) Review request for mesos and Ben Mahler. Changes --- Addressed style issues reported by build bot. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs (updated) - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 26052: Add docker container prefix to docker docs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26052/#review54585 --- Ship it! Ship It! - Niklas Nielsen On Sept. 25, 2014, 12:37 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26052/ --- (Updated Sept. 25, 2014, 12:37 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/26052 Diffs - docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c Diff: https://reviews.apache.org/r/26052/diff/ Testing --- Thanks, Timothy Chen
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54588 --- Ship it! Ship It! - Niklas Nielsen On Sept. 22, 2014, 6:18 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 22, 2014, 6:18 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/src/process.cpp 8adc809 3rdparty/libprocess/src/process_reference.hpp PRE-CREATION Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26056: Prevent multiple definitions of synchronized header.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/#review54596 --- 3rdparty/libprocess/src/synchronized.hpp https://reviews.apache.org/r/26056/#comment94777 I know it is pretty inconsistent in libprocess, but let's surround the include guard with '__' and do a scan of the other headers too - Niklas Nielsen On Sept. 25, 2014, 2 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/ --- (Updated Sept. 25, 2014, 2 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Defend against multiple definitions of synchronized. Add conditional include. Diffs - 3rdparty/libprocess/src/synchronized.hpp 577a5fc Diff: https://reviews.apache.org/r/26056/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25986: Added reconcileTasks to python scheduler.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25986/ --- (Updated Sept. 25, 2014, 9:09 p.m.) Review request for mesos and Ben Mahler. Changes --- Linked in MESOS-1461. --bmahler Bugs: MESOS-1461 https://issues.apache.org/jira/browse/MESOS-1461 Repository: mesos-git Description --- The last step of wiring up reconcileTasks in the python bindings. Diffs - src/python/interface/src/mesos/interface/__init__.py 818f41b Diff: https://reviews.apache.org/r/25986/diff/ Testing --- Functional testing Thanks, Niklas Nielsen
Re: Review Request 25986: Added reconcileTasks to python scheduler.
On Sept. 25, 2014, 2:08 p.m., Ben Mahler wrote: Great, didn't realize this was all that's needed for MESOS-1461, I'll assign that to you. Can you link the ticket into this review? Yeah - wired everything up but the last step :-P We should wire task reconciliation up in the test frameworks to exercise the api. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25986/#review54597 --- On Sept. 25, 2014, 2:09 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25986/ --- (Updated Sept. 25, 2014, 2:09 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1461 https://issues.apache.org/jira/browse/MESOS-1461 Repository: mesos-git Description --- The last step of wiring up reconcileTasks in the python bindings. Diffs - src/python/interface/src/mesos/interface/__init__.py 818f41b Diff: https://reviews.apache.org/r/25986/diff/ Testing --- Functional testing Thanks, Niklas Nielsen
Re: Review Request 26056: Normalize enforce 3rdparty/libprocess conditional include style.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/#review54625 --- Ship it! Ship It! - Niklas Nielsen On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/ --- (Updated Sept. 25, 2014, 4:04 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Normalize 3rdparty/libprocess conditional include style. Diffs - 3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 3rdparty/libprocess/src/config.hpp cbaf41d 3rdparty/libprocess/src/fatal.hpp 38646f3 3rdparty/libprocess/src/net.hpp 1ba3dbb 3rdparty/libprocess/src/process_reference.hpp 4964107 3rdparty/libprocess/src/synchronized.hpp 577a5fc Diff: https://reviews.apache.org/r/26056/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26056: Normalize enforce 3rdparty/libprocess conditional include style.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/#review54626 --- 3rdparty/libprocess/src/net.hpp https://reviews.apache.org/r/26056/#comment94856 Missed this one s/HH/HPP/ - Niklas Nielsen On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/ --- (Updated Sept. 25, 2014, 4:04 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Normalize 3rdparty/libprocess conditional include style. Diffs - 3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 3rdparty/libprocess/src/config.hpp cbaf41d 3rdparty/libprocess/src/fatal.hpp 38646f3 3rdparty/libprocess/src/net.hpp 1ba3dbb 3rdparty/libprocess/src/process_reference.hpp 4964107 3rdparty/libprocess/src/synchronized.hpp 577a5fc Diff: https://reviews.apache.org/r/26056/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26056: Normalize enforce 3rdparty/libprocess conditional include style.
On Sept. 25, 2014, 4:35 p.m., Niklas Nielsen wrote: 3rdparty/libprocess/src/net.hpp, line 232 https://reviews.apache.org/r/26056/diff/2/?file=705786#file705786line232 Missed this one s/HH/HPP/ Fixed it when committing - thanks! - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/#review54626 --- On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26056/ --- (Updated Sept. 25, 2014, 4:04 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Normalize 3rdparty/libprocess conditional include style. Diffs - 3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 3rdparty/libprocess/src/config.hpp cbaf41d 3rdparty/libprocess/src/fatal.hpp 38646f3 3rdparty/libprocess/src/net.hpp 1ba3dbb 3rdparty/libprocess/src/process_reference.hpp 4964107 3rdparty/libprocess/src/synchronized.hpp 577a5fc Diff: https://reviews.apache.org/r/26056/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25848: Introducing mesos modules.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/#review54879
---
Style review.
One high-level comment: the additional fields for authoring doesn't seem to
scale well - how about introducing a struct for that payload. In other words,
let's try to limit the use of macros to a bare minimum.
include/mesos/module.hpp.in
https://reviews.apache.org/r/25848/#comment95221
Can we use existing boost helpers for this? Something like
http://www.boost.org/doc/libs/1_56_0/libs/preprocessor/doc/ref/cat.html
include/mesos/module.hpp.in
https://reviews.apache.org/r/25848/#comment95222
You can wrap all the functions in extern C {}
src/Makefile.am
https://reviews.apache.org/r/25848/#comment95193
Kill spaces - hard tabs in makefiles
src/examples/test_module.hpp
https://reviews.apache.org/r/25848/#comment95194
{ on new line
src/examples/test_module_impl.cpp
https://reviews.apache.org/r/25848/#comment95195
newline between includes from mesos, stout, ...
src/examples/test_module_impl.cpp
https://reviews.apache.org/r/25848/#comment95196
Wrap according to
http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/
src/examples/test_module_impl2.cpp
https://reviews.apache.org/r/25848/#comment95197
Newline between the includes
src/examples/test_module_impl2.cpp
https://reviews.apache.org/r/25848/#comment95198
Can we avoid complex types in global scope?
And/or have const strings at least?
src/examples/test_module_impl2.cpp
https://reviews.apache.org/r/25848/#comment95199
You can mark the whole block (all the MESOS_ macros) as extern C to
avoid duplication:
extern C {
...
}
Opposed to extern C every time
src/module/manager.hpp
https://reviews.apache.org/r/25848/#comment95224
Any particular reason for both using hashmap and maps?
src/module/manager.hpp
https://reviews.apache.org/r/25848/#comment95209
const string? Here and below
src/module/manager.hpp
https://reviews.apache.org/r/25848/#comment95219
can we have const strings as keys?
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95207
If we wanted to be pedantic from the start, how about setting all to
MESOS_VERSION (like we discussed in the dev@ mailing thread) until we get
comforatble with the api's stability?
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95205
Mind indent according to
http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ ?
Bring down on newline and 4 space indent
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95206
Same here
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95204
Two newlines.
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95203
Still a bit too dense - can we break up and comment on how it applies to
the format defined in the header?
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95225
Two newlines.
src/module/manager.cpp
https://reviews.apache.org/r/25848/#comment95217
can we have the key in libraryToModules be a const string instead?
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95215
Newline between the two
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95202
Two newlines between implementing functions
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95210
Can we have it return a const string?
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95201
bring this down on newline and indent 2
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95211
See above - let's work a bit on constness of the strings here and below.
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95213
const string version?
src/tests/module_tests.cpp
https://reviews.apache.org/r/25848/#comment95214
const strings?
- Niklas Nielsen
On Sept. 29, 2014, 2:16 p.m., Kapil Arya wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/
---
(Updated Sept. 29, 2014, 2:16 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen,
and Timothy St. Clair.
Bugs: MESOS-1384
https://issues.apache.org/jira/browse/MESOS-1384
Repository: mesos-git
Description
---
Adding a first class primitive, abstraction and process for dynamic library
writing and loading can make it easier to extend inner workings of Mesos.
Making it possible to have dynamic loadable resource allocators, isolators
Re: Review Request 19180: Fix mesos command parsing help
On May 15, 2014, 4:32 p.m., Niklas Nielsen wrote: Hey Chengwei - sorry for the tardy turnaround time on this review request. To me, it still seems like we are treating the symptoms of the real issue: PATH is appended multiple times and the subsequent globbing adds the available commands to same number of times. The reason I am saying this is because the fix is difficult to understand (it is not immediate that this is the problem it solves) and seems very specialized for the mesos help --help and mesos help help case. Two things we could do: 1) Don't add the new path unconditionally to the PATH variable i.e. check if it is already there. 2) In usage(), don't add duplicates to the commands from the globbed list of candidates. This can be done pretty easy and local by using a set instead of a list. Try to take a look at: https://github.com/nqn/mesos/commit/01f77a1ab6d96f48765cc3539da1a1ebd4ba1d56 Thoughts? Adam B wrote: +1 Love the 'set'. Calling mesos help foo will still recurse into main and dirname will still be prepended to PATH multiple times, but the commands will not be printed multiple times. mesos help help will give a weird error (Not expecting --help before command) instead of calling usage, but I think that's a pretty contrived case. Chengwei Yang wrote: Hi Niklas, Sorry for late reply, so since the 2) improvement landed into usage(), so anyway we can't get duplicated commands in usage now though the 1) thing is still left to take. Do you like the first version of this patch? Which just do the small fix, add directory to PATH in the first through. Niklas Nielsen wrote: Can you point me review there 2) landed? If that's is in, why bother with 1)? I am still _not_ in favor of a notion of firstThrough with a static variable - if anything, it should be firstPass and I already enumerated other ways of doing it. If you want to push for that fix still, I suggest we find another shepherd for this RR. Chengwei Yang wrote: Sorry, I didn't aware that patch is still in your branch, not the official mesos repo. Do you submit that patch recently? Even we have your patch merged, the issue (directory added into $PATH more than once) is still here. I'll align this patch with your vision soon. Hi Chengwei - do you still want this to go in? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19180/#review43181 --- On May 15, 2014, 3:48 p.m., Chengwei Yang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19180/ --- (Updated May 15, 2014, 3:48 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-1093 https://issues.apache.org/jira/browse/MESOS-1093 Repository: mesos-git Description --- Fix mesos command parsing help Without this patch, mesos help --help will output below Not expecting '--help' before command Usage: mesos command [OPTIONS] Available commands: help resolve cat scp log tail execute ps local resolve cat scp log tail execute ps local Apparently all available commands printed twice, the mesos help help will print available commands 3 times. The root cause is the directory contains available mesos commands are added more than one times when recursive to main() again. Idea comes from Adam B. Review: https://reviews.apache.org/r/19180 Diffs - src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 Diff: https://reviews.apache.org/r/19180/diff/ Testing --- done? Thanks, Chengwei Yang
Re: Review Request 26125: Add webui_url to FrameworkInfo and web UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26125/#review55010 --- Vinod Tobi: I can help out landing this if you are interested. I can't claim to be an angular expert though. One high-level comment: how about we write a test so we make sure it works in the future too? A simple framework register + parse state.json would do. src/webui/master/static/framework.html https://reviews.apache.org/r/26125/#comment95343 Will this be shown no matter if the framework announced a framework webui or not? Can we make it conditional like below? - Niklas Nielsen On Sept. 27, 2014, 9:28 p.m., Tobi Knaup wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26125/ --- (Updated Sept. 27, 2014, 9:28 p.m.) Review request for mesos. Bugs: MESOS-1828 https://issues.apache.org/jira/browse/MESOS-1828 Repository: mesos-git Description --- Allows frameworks to register a URL to its web UI. Diffs - include/mesos/mesos.proto be45494 src/master/http.cpp 41d91c8 src/webui/master/static/framework.html f6cce02 src/webui/master/static/frameworks.html b11d71a Diff: https://reviews.apache.org/r/26125/diff/ Testing --- Connected a framework with and without webui_url set and checked the web ui. Thanks, Tobi Knaup
Re: Review Request 25785: Add guide to becoming a committer
On Sept. 19, 2014, 5:24 p.m., Adam B wrote: I think it's valuable to outline some guidelines for anybody interested in becoming a committer, but I don't think we should set too many strict rules and regulations around it. Other thoughts below. Dominic Hamon wrote: I think exactly the opposite. I think clear guidelines give contributors goals to attain and allow them to have an aim further than just contributing to the project. It also reduces friction when deciding who to nominate as a new committer and encourages the nomination of new committers as contributors mature. Taken the number of different views on the topic - isn't it worthwhile to bring to the dev@ list for discussion rather then iterating on a patch? Or have a google doc for mutual commenting / editing? I also have a hard time getting comfortable with establishing a process that haven't been up for vote (but written down anyway) - to put it another way: how can we honor/exercise these rules if there haven't been consensus around them? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/#review54053 --- On Sept. 18, 2014, 10:41 a.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/ --- (Updated Sept. 18, 2014, 10:41 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-1815 https://issues.apache.org/jira/browse/MESOS-1815 Repository: mesos-git Description --- Add a guide to becoming a committer. Diffs - docs/becoming-a-committer.md PRE-CREATION Diff: https://reviews.apache.org/r/25785/diff/ Testing --- Thanks, Dominic Hamon
Re: Review Request 22169: Added External Containerizer documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22169/#review55024 --- Fly by review - it is much better than the current (non-existing) docs on the EC. So I think we should land this asap :-) Tom, do you have cycles to review? docs/external-containerizer.md https://reviews.apache.org/r/22169/#comment95357 Worth making the paths relative? - Niklas Nielsen On Sept. 5, 2014, 1:48 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22169/ --- (Updated Sept. 5, 2014, 1:48 a.m.) Review request for mesos and Tom Arnfeld. Repository: mesos-git Description --- Adds a markdown document describing the ExternalContainerizer. Diffs - docs/external-containerizer.md PRE-CREATION docs/home.md 179a164 docs/images/ec_kill_seqdiag.png PRE-CREATION docs/images/ec_launch_seqdiag.png PRE-CREATION docs/images/ec_lifecycle_seqdiag.png PRE-CREATION docs/images/ec_orphan_seqdiag.png PRE-CREATION docs/images/ec_recover_seqdiag.png PRE-CREATION Diff: https://reviews.apache.org/r/22169/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 25848: Introducing mesos modules.
On Oct. 1, 2014, 1:35 p.m., Benjamin Hindman wrote: src/module/manager.cpp, line 59 https://reviews.apache.org/r/25848/diff/12/?file=709886#file709886line59 Maybe I'm just getting a big grumpy, but I'm really not in favor of overloading the term 'role' here. We've overloaded terms a handful of times in Mesos/libprocess and it has never turned out well. Moreover, by calling this a 'role' we're somehow implying that it's something more than just a type, yet when I look at the possible roles suggested here these are just types, Isolator, Authenticator, Allocator, etc. What's the benefit of introducing a new term for this that we need to define for people and they'll have to remember? Kapil Arya wrote: Bernd/Nik: We have been talking about finding a better name anyways. Any ideas? I suggested 'kind' before - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review55065 --- On Oct. 1, 2014, 4:18 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 1, 2014, 4:18 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - configure.ac 86d448c3ad00ad01d3d069c1039dc7ad524af567 include/mesos/module.hpp.in PRE-CREATION src/Makefile.am e588fd0298f43e44ba01a2b0c907145b801ff1c2 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/examples/test_module_impl2.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 26125: Add webui_url to FrameworkInfo and web UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26125/#review55229 --- Ship it! Ship It! - Niklas Nielsen On Oct. 2, 2014, 9:10 a.m., Tobi Knaup wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26125/ --- (Updated Oct. 2, 2014, 9:10 a.m.) Review request for mesos. Bugs: MESOS-1828 https://issues.apache.org/jira/browse/MESOS-1828 Repository: mesos-git Description --- Allows frameworks to register a URL to its web UI. Diffs - include/mesos/mesos.proto be45494 src/master/http.cpp 41d91c8 src/tests/master_tests.cpp 1497db2 src/webui/master/static/framework.html f6cce02 src/webui/master/static/frameworks.html b11d71a Diff: https://reviews.apache.org/r/26125/diff/ Testing --- Connected a framework with and without webui_url set and checked the web ui. Thanks, Tobi Knaup
Re: Review Request 26303: Move Mesos version from mesos.hpp to version.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26303/#review55305 --- Ship it! Ship It! - Niklas Nielsen On Oct. 2, 2014, 5:26 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26303/ --- (Updated Oct. 2, 2014, 5:26 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- This allows the Module subsystem to simply include mesos/version.hpp instead of inheriting the whole protobuf header chain. Diffs - configure.ac 86d448c3ad00ad01d3d069c1039dc7ad524af567 include/mesos/mesos.hpp.in 8aedf8c4d27014c3e61eac801d6267b3d316b38a include/mesos/version.hpp.in PRE-CREATION src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b Diff: https://reviews.apache.org/r/26303/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
On Sept. 29, 2014, 2:57 p.m., Niklas Nielsen wrote: include/mesos/module.hpp.in, lines 46-47 https://reviews.apache.org/r/25848/diff/11/?file=708522#file708522line46 Can we use existing boost helpers for this? Something like http://www.boost.org/doc/libs/1_56_0/libs/preprocessor/doc/ref/cat.html Kapil Arya wrote: Right now this files doesn't #include any other files and since its just one definition, it seemed simpler to just define APPEND and EXPAND_AND_APPEND macros. If there is a strong preference, we can definitely use BOOST_PP_CAT instead. Timothy St. Clair wrote: 2¢ - Don't add extra inclusion. Feel free to drop - the new struct scheme should get rid of it. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54879 --- On Oct. 1, 2014, 4:18 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 1, 2014, 4:18 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - configure.ac 86d448c3ad00ad01d3d069c1039dc7ad524af567 include/mesos/module.hpp.in PRE-CREATION src/Makefile.am e588fd0298f43e44ba01a2b0c907145b801ff1c2 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/examples/test_module_impl2.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.
On Oct. 2, 2014, 12:49 a.m., Timothy Chen wrote: src/slave/utils.cpp, line 39 https://reviews.apache.org/r/25434/diff/4/?file=709161#file709161line39 Log the timeout value Both the one that is too small and the effective one. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review55196 --- On Oct. 2, 2014, 12:17 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Oct. 2, 2014, 12:17 a.m.) Review request for mesos, Benjamin Hindman, Niklas Nielsen, Till Toenshoff, and Timothy St. Clair. Bugs: MESOS-1571 https://issues.apache.org/jira/browse/MESOS-1571 Repository: mesos-git Description --- The configurable slave's executor_shutdown_grace_period flag is propagated to Executor and CommandExecutor through an environment variable. Shutdown timeout in Executor and signal escalation timeout in CommandExecutor are now dependent on this flag. Each nested timeout is somewhat shorter than the parent one. Diffs - src/Makefile.am 27c42df src/exec/exec.cpp e15f834 src/launcher/executor.cpp cbc8750 src/slave/constants.hpp 9030871 src/slave/constants.cpp e1da5c0 src/slave/containerizer/containerizer.hpp 8a66412 src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/docker.cpp 9a29489 src/slave/containerizer/external_containerizer.cpp efbc68f src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/slave/flags.hpp 32e51d2 src/slave/utils.hpp PRE-CREATION src/slave/utils.cpp PRE-CREATION src/tests/containerizer.cpp a17e1e0 src/tests/mesos.hpp 957e223 src/tests/slave_tests.cpp 69be28f Diff: https://reviews.apache.org/r/25434/diff/ Testing --- make check (OS X 10.9.4) WIP: digging into the failure of the SlaveTest.MesosExecutorForceShutdown test revealed an issue with signal escalation in CommandExecutor. That needs more time to be resolved. Thanks, Alexander Rukletsov
Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review55348 --- Hi Alex, Sorry for the tardy reply on this review. I have left a few high-level comments - think that Tim touched upon most of the style issues. src/exec/exec.cpp https://reviews.apache.org/r/25434/#comment95718 Why Java executors? I am not sure I understand this comment. src/slave/constants.hpp https://reviews.apache.org/r/25434/#comment95726 Can we add something similar to the flags help text? The flag is already a bit misleading, so it would be helpful to expand on the new funtionality there. src/slave/utils.hpp https://reviews.apache.org/r/25434/#comment95715 Can you include an example? How about the sequence charts from the JIRA? The logic of adjustShutdownTimeout is still not crystal clear to me. To put it in another way; it is hard for me to tell whether we cover all corner cases. src/slave/utils.hpp https://reviews.apache.org/r/25434/#comment95714 How about 'getShutdownTimeout'? It is not really adjusting, but rather partition the total timeout into sub-timeouts - correct? Also, we can hide the implementation adjustShutdownTimeout in utils.cpp so we only export adjustExecutorShutdownTimeout and adjustCommandExecutorShutdownTimeout (which could be called 'getExecutorShutdownTimeout'?) src/slave/utils.cpp https://reviews.apache.org/r/25434/#comment95716 Do we want to have checks/asserts to ensure we have positive/non-zero timeouts? src/tests/mesos.hpp https://reviews.apache.org/r/25434/#comment95717 You only use this in MesosExecutorForceShutdown - how about moving it to slave_tests.cpp instead? How about using 'statusMatchesTask' or 'statusHasTaskId'. 'relatedTo' seems a bit informal/imprecise. - Niklas Nielsen On Oct. 2, 2014, 12:17 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Oct. 2, 2014, 12:17 a.m.) Review request for mesos, Benjamin Hindman, Niklas Nielsen, Till Toenshoff, and Timothy St. Clair. Bugs: MESOS-1571 https://issues.apache.org/jira/browse/MESOS-1571 Repository: mesos-git Description --- The configurable slave's executor_shutdown_grace_period flag is propagated to Executor and CommandExecutor through an environment variable. Shutdown timeout in Executor and signal escalation timeout in CommandExecutor are now dependent on this flag. Each nested timeout is somewhat shorter than the parent one. Diffs - src/Makefile.am 27c42df src/exec/exec.cpp e15f834 src/launcher/executor.cpp cbc8750 src/slave/constants.hpp 9030871 src/slave/constants.cpp e1da5c0 src/slave/containerizer/containerizer.hpp 8a66412 src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/docker.cpp 9a29489 src/slave/containerizer/external_containerizer.cpp efbc68f src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/slave/flags.hpp 32e51d2 src/slave/utils.hpp PRE-CREATION src/slave/utils.cpp PRE-CREATION src/tests/containerizer.cpp a17e1e0 src/tests/mesos.hpp 957e223 src/tests/slave_tests.cpp 69be28f Diff: https://reviews.apache.org/r/25434/diff/ Testing --- make check (OS X 10.9.4) WIP: digging into the failure of the SlaveTest.MesosExecutorForceShutdown test revealed an issue with signal escalation in CommandExecutor. That needs more time to be resolved. Thanks, Alexander Rukletsov
Re: Review Request 25614: Safer handling of futures in JNI state abstraction
On Sept. 15, 2014, 2:17 p.m., Jie Yu wrote: This more looks like a bug somewhere. Let's try to find out the true root cause first. Niklas Nielsen wrote: True - but throwing the result away from await() seems silly/unsafe too (rather on relying on the await to have completed or wait forever). Our investigation ended in https://github.com/apache/mesos/blob/master/3rdparty/libprocess/src/latch.cpp#L58 (as explained in the RR description). We haven't been able to realiably reproduce the fault to debug it - Connor, can you fill in on when it happened in Marathon? Connor Doyle wrote: Sure, will try to reproduce with more verbose logging on a fresh cluster tomorrow. Ben Mahler wrote: The semantics of Future::await() are such that if no Duration is provided, then it will never return false: ``` f.await(); // Always returns true. // Now the future should not be pending! ``` It should only return false as the result of a timeout. That's why Jie is saying we need to understand the bug, it's not even clear whether await returned false (which should not occur) vs. returned true but the CHECK_READY still saw the future as pending. Connor, do you want to close this issue for now and continue the investigation in the JIRA ticket? We can reopen if this is indeed the issue. Thanks - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53409 --- On Sept. 15, 2014, 1:54 p.m., Connor Doyle wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/ --- (Updated Sept. 15, 2014, 1:54 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-1795 https://issues.apache.org/jira/browse/MESOS-1795 Repository: mesos-git Description --- ## OVERVIEW This change adds a measure of safety for calls to `FutureT::await()` without a timeout from within the JNI interface to the state abstraction. ## MOTIVATION Observed the following log output prior to a crash of the Marathon scheduler: ``` Sep 12 23:46:01 highly-available-457-540 marathon[11494]: F0912 23:46:01.771927 11532 org_apache_mesos_state_AbstractState.cpp:145] CHECK_READY(*future): is PENDING Sep 12 23:46:01 highly-available-457-540 marathon[11494]: *** Check failure stack trace: *** Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663a2d google::LogMessage::Fail() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26657e3 google::LogMessage::SendToLog() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc2663648 google::LogMessage::Flush() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc266603e google::LogMessageFatal::~LogMessageFatal() Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febc26588a3 Java_org_apache_mesos_state_AbstractState__1_1fetch_1get Sep 12 23:46:01 highly-available-457-540 marathon[11494]: @ 0x7febcd107d98 (unknown) ``` _Listing 1: Crash log output._ ## CHANGES IN DETAIL The in-line comments in `Latch::await(const Duration duration)`, mention that it's possible for the call to return before the underlying process has completed for two reasons: ``` ... process::wait(pid, duration); // Explict to disambiguate. // It's possible that we failed to wait because: // (1) Our process has already terminated. // (2) We timed out (i.e., duration was not infinite). ... ``` _Listing 2: Excerpt from 3rdparty/src/libprocess/latch.cpp._ Call sites within `org_apache_mesos_state_AbstractState.cpp` that formerly discarded the return value now throw a `java.concurrent.ExecutionException` if the await fails. I chose to throw this instead of a `java.concurrent.TimeoutException` since the expectation from the caller's perspective is to block forever pending a ready state. Diffs - src/java/jni/org_apache_mesos_state_AbstractState.cpp 1accc8a Diff: https://reviews.apache.org/r/25614/diff/ Testing --- - make - make check Thanks, Connor Doyle
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review55823 --- Ship it! Ship It! - Niklas Nielsen On Oct. 8, 2014, 9:22 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 8, 2014, 9:22 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am c62a974dcc80f3c3dd6060aee51f8367a0abe724 src/examples/example_module_impl.cpp PRE-CREATION src/examples/test_module.hpp PRE-CREATION src/messages/messages.proto 9ff06b38086010df362036c695a5222371f70f4d src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/master_tests.cpp 705e5f2c0f9d693946e722cef63f38f3bff0d46b src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 26150: Libprocess Benchmark
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/#review55824
---
A high-level comment: It is a bit hard to understand what's going on here. The
test body is pretty dense and not well commented - mind spending a few cycles
breaking it up and make it more consumable?
Also, we need to do a style scan :-)
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96201
Let's include the usual license blob.
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96203
We try to use camel case, so _numIter, _maxOutstanding and so on.
Here and below.
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96202
Out of curiosity - wouldn't plain '0' do?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96204
Bring '{' down on new line
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96205
We try to spell out variable names. How about message?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96206
s/max_outstanding/maxOutstanding/
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96208
Instead of doing char* and strlen. Can we use a std::string instead?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96210
For wrapping boolean expressions, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Boolean_Expressions
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96214
Would you mind adding a comment on what the test is doing here?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96211
+1 - can we do this the old way for now?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96212
Same here
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96213
I am not sure we have graced range based loops yet. Do you have any
references to whether it is supported by our set of graced compilers?
- Niklas Nielsen
On Sept. 29, 2014, 2:38 p.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/
---
(Updated Sept. 29, 2014, 2:38 p.m.)
Review request for mesos and Niklas Nielsen.
Bugs: MESOS-1840
https://issues.apache.org/jira/browse/MESOS-1840
Repository: mesos-git
Description
---
A benchmark for libprocess.
It forks num_proc times and launched num_threads libprocess processes in
each child. They are aware of the master's (parent) address and all play
ping pong with it.
This benchmark measures throughput in terms of the number of RPCs handled per
second using persistent (linked) connections.
A new test file (benchmarks) is introduced because we want to fork before
libprocess is initialized. This allows us to prevent short-circuiting of
message passing between processes under the same ProcessManager. This way we
force the execution path of the underlying event management system.
Diffs
-
3rdparty/libprocess/Makefile.am 616618e
3rdparty/libprocess/src/tests/benchmarks.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26150/diff/
Testing
---
make check in 3rdparty/libprocess
support/mesos-style.py
Thanks,
Joris Van Remoortere
Re: Review Request 23710: Add line comments end punctuation style rule
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23710/#review55831 --- Hey Tim, did you get to address Adam's comment on the off-by-one issue? - Niklas Nielsen On Aug. 25, 2014, 5:12 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23710/ --- (Updated Aug. 25, 2014, 5:12 p.m.) Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/23710 Diffs - support/cpplint.patch 1dd69a03044c0c17ca4ee555c6c9d27ea043d4f0 support/cpplint.py bfd3390002a680b07aa3fcf785279ad19625294b support/mesos-style.py d24cb11adc06bc0ebaaa206301616c8b597f09e8 Diff: https://reviews.apache.org/r/23710/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review55874 --- src/module/manager.cpp https://reviews.apache.org/r/25848/#comment96244 Just noticed this - shutdown() doesn't describe what you expect the function to do or how you use it. How about ::clear() or ::unloadAll()? - Niklas Nielsen On Oct. 8, 2014, 9:22 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 8, 2014, 9:22 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am c62a974dcc80f3c3dd6060aee51f8367a0abe724 src/examples/example_module_impl.cpp PRE-CREATION src/examples/test_module.hpp PRE-CREATION src/messages/messages.proto 9ff06b38086010df362036c695a5222371f70f4d src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/master_tests.cpp 705e5f2c0f9d693946e722cef63f38f3bff0d46b src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 26467: Replaced hard tabs with spaces in module/manager.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26467/#review55896 --- Ship it! Ship It! - Niklas Nielsen On Oct. 8, 2014, 4:33 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26467/ --- (Updated Oct. 8, 2014, 4:33 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Replaced hard tabs with spaces in module/manager.cpp. Diffs - src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc Diff: https://reviews.apache.org/r/26467/diff/ Testing --- Thanks, Kapil Arya
Re: Build failed in Jenkins: mesos-reviewbot #1900
Fixed in https://reviews.apache.org/r/26467/ - sorry for the noise
Niklas
On 8 October 2014 16:32, Apache Jenkins Server jenk...@builds.apache.org
wrote:
See https://builds.apache.org/job/mesos-reviewbot/1900/
--
[URLTrigger] A change within the response URL invocation (log)
Building remotely on ubuntu-5 (docker Ubuntu ubuntu5 ubuntu) in workspace
https://builds.apache.org/job/mesos-reviewbot/ws/
git rev-parse --is-inside-work-tree
Fetching changes from the remote Git repository
git config remote.origin.url
https://git-wip-us.apache.org/repos/asf/mesos.git
Fetching upstream changes from
https://git-wip-us.apache.org/repos/asf/mesos.git
git --version
git fetch --tags --progress
https://git-wip-us.apache.org/repos/asf/mesos.git
+refs/heads/*:refs/remotes/origin/*
git rev-parse origin/master^{commit}
Checking out Revision 93701846c88e1da3f45cb7b14b54d81f9f3bc68f
(origin/master)
git config core.sparsecheckout
git checkout -f 93701846c88e1da3f45cb7b14b54d81f9f3bc68f
git rev-list 93701846c88e1da3f45cb7b14b54d81f9f3bc68f
git tag -a -f -m Jenkins Build #1900 jenkins-mesos-reviewbot-1900
[mesos-reviewbot] $ /bin/bash -xe /tmp/hudson1051060795229853582.sh
+ export JAVA_HOME=/home/jenkins/tools/java/jdk1.6.0_20-64
+ JAVA_HOME=/home/jenkins/tools/java/jdk1.6.0_20-64
+ export
PATH=/home/jenkins/tools/java/jdk1.6.0_20-64/bin:/home/jenkins/tools/java/latest1.6/bin:/home/jenkins/tools/java/latest1.6/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
+
PATH=/home/jenkins/tools/java/jdk1.6.0_20-64/bin:/home/jenkins/tools/java/latest1.6/bin:/home/jenkins/tools/java/latest1.6/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
+ export M2_HOME=/home/jenkins/tools/maven/latest
+ M2_HOME=/home/jenkins/tools/maven/latest
+ export
PATH=/home/jenkins/tools/maven/latest/bin:/home/jenkins/tools/java/jdk1.6.0_20-64/bin:/home/jenkins/tools/java/latest1.6/bin:/home/jenkins/tools/java/latest1.6/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
+
PATH=/home/jenkins/tools/maven/latest/bin:/home/jenkins/tools/java/jdk1.6.0_20-64/bin:/home/jenkins/tools/java/latest1.6/bin:/home/jenkins/tools/java/latest1.6/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
+ date
Wed Oct 8 23:31:47 UTC 2014
+ chmod -R +w 3rdparty bin bootstrap CHANGELOG configure.ac docs Doxyfile
ec2 frameworks include LICENSE m4 Makefile.am mesos-0.20.0 mesos.pc.in
mpi NOTICE README.md src support
+ git clean -fdx
Skipping repository mesos-0.20.0/_build/3rdparty/leveldb
+ git reset --hard HEAD
HEAD is now at 9370184 Added style/syntax changes made to modules
abstractions.
+ ./support/mesos-style.py
Checking 518 files using filter
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/module/manager.cpp:194: Tab found; better to use spaces
[whitespace/tab] [1]
src/module/manager.cpp:199: Tab found; better to use spaces
[whitespace/tab] [1]
src/module/manager.cpp:200: Tab found; better to use spaces
[whitespace/tab] [1]
Total errors found: 3
Build step 'Execute shell' marked build as failure
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56013 --- src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96366 s/The value could be a/A/ ? src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96367 .. that will be loaded and accessible to augment mesos subsystems? Something that describes what happens to the input :-) src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96372 Or else what? Modules gets overwritten right? Or maybe just mentioned that you 'must not'? src/module/manager.hpp https://reviews.apache.org/r/26508/#comment96373 Is this just a fly-by style fix? Why include module.hpp here? - Niklas Nielsen On Oct. 9, 2014, 10:19 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 10:19 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56024 --- src/slave/flags.hpp https://reviews.apache.org/r/26509/#comment96383 Let's get the master flags help text solidified and update this accordingly. - Niklas Nielsen On Oct. 9, 2014, 10:20 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 9, 2014, 10:20 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26513: Disallow duplicate module names.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/#review56043 --- Ship it! Looks good! Will get this committed for you shortly - Niklas Nielsen On Oct. 9, 2014, 12:59 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/ --- (Updated Oct. 9, 2014, 12:59 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Module manager should return error on encountering a duplicate module name (i.e. if a module with the same name has already been loaded). Diffs - src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26513/diff/ Testing --- Added a test with duplicate modules; ran make check. Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 --- src/module/manager.cpp https://reviews.apache.org/r/26529/#comment96471 Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) src/tests/module_tests.cpp https://reviews.apache.org/r/26529/#comment96470 Let's try to avoid macro constants - can we do static const char* or define a helper that does this for you? src/tests/module_tests.cpp https://reviews.apache.org/r/26529/#comment96472 const strings if you don't intent to change them How about hoisting the 'LD_LIBRARY_PATH' string and define it once as a constant? - Niklas Nielsen On Oct. 10, 2014, 8:24 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 8:24 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
On Oct. 10, 2014, 8:31 a.m., Niklas Nielsen wrote: src/module/manager.cpp, lines 191-193 https://reviews.apache.org/r/26529/diff/2/?file=717123#file717123line191 Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) Kapil Arya wrote: Should I create a separate review request for the empty test(s)? One or the other. The commit didn't include any details on the bug you fixed when renaming 'path' - I will leave that up to you. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 --- On Oct. 10, 2014, 8:24 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 8:24 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56157 --- src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96476 Can we include the JSON syntax here instead of '...'? - Niklas Nielsen On Oct. 9, 2014, 4:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 4:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56183 --- Ship it! Ship It! - Niklas Nielsen On Oct. 9, 2014, 4:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 4:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56237 --- Ship it! Ship It! - Niklas Nielsen On Oct. 10, 2014, 2:32 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 2:32 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Also, return a better error message if an empty module name is provided. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26588: Fixed small typo in master module flags description.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/#review56268 --- Ship it! Ship It! - Niklas Nielsen On Oct. 10, 2014, 5:41 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/ --- (Updated Oct. 10, 2014, 5:41 p.m.) Review request for mesos, Kapil Arya and Niklas Nielsen. Repository: mesos-git Description --- see summary. Diffs - src/master/flags.hpp 44249f8 Diff: https://reviews.apache.org/r/26588/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 26150: Libprocess Benchmark
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/#review56442
---
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96773
This is the first time we use std::thread - do you have some references
that it is supported across our graced compilers? Same for mutex
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96775
2 newlines between implementing functions.
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96771
This is a bit counter intuitive (from the 4 space argument wrapping), but
for intializer lists take a look at for example:
https://github.com/apache/mesos/blob/master/src/docker/docker.hpp#L65
It becomes:
FooBarBaz(
int x,
int y,
int z)
: x(x),
y(y),
z(z)
{
}
Does that make sense?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96772
setLink?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96774
Bring '{' down - here and below.
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96776
Was this the lock that depended on Dominic's configure patch? If so, can
you add it as a dependency?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96781
Can we use the latch (latch.hpp) abstraction here?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96777
Mind throwing in a comment here on why you need this helper (thread entry?)
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96760
Any reason not to use plain int's?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96751
Why int64? It is only a count down from 4 right?
Also, shouldn't moreToLaunch be size_t to be consistent?
3rdparty/libprocess/src/tests/benchmarks.cpp
https://reviews.apache.org/r/26150/#comment96778
Stout has a time abstraction you could use:
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/time.hpp
- Niklas Nielsen
On Oct. 13, 2014, 1:38 p.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26150/
---
(Updated Oct. 13, 2014, 1:38 p.m.)
Review request for mesos and Niklas Nielsen.
Bugs: MESOS-1840
https://issues.apache.org/jira/browse/MESOS-1840
Repository: mesos-git
Description
---
A benchmark for libprocess.
It forks num_proc times and launched num_threads libprocess processes in
each child. They are aware of the master's (parent) address and all play
ping pong with it.
This benchmark measures throughput in terms of the number of RPCs handled per
second using persistent (linked) connections.
A new test file (benchmarks) is introduced because we want to fork before
libprocess is initialized. This allows us to prevent short-circuiting of
message passing between processes under the same ProcessManager. This way we
force the execution path of the underlying event management system.
Diffs
-
3rdparty/libprocess/Makefile.am 616618e
3rdparty/libprocess/src/tests/benchmarks.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26150/diff/
Testing
---
make check in 3rdparty/libprocess
support/mesos-style.py
Thanks,
Joris Van Remoortere
Re: Review Request 17431: Enabled configuration of the mesos master from the UI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17431/#review56533 --- The latest diff looks broken - Thomas, mind updating it (if you still want this to go in)? - Niklas Nielsen On Jan. 29, 2014, 12:51 p.m., Thomas Rampelberg wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17431/ --- (Updated Jan. 29, 2014, 12:51 p.m.) Review request for mesos and Ross Allen. Bugs: mesos-885 https://issues.apache.org/jira/browse/mesos-885 Repository: mesos-git Description --- Enabled configuration of the mesos master from the UI. Review: http://reviews.apache.org/r/17431 Diffs - CHANGELOG e75a3411f865cb7f3768df1299f469f49c3a0009 bin/lldb-mesos-local.sh.in 35011fd483e477701efd7e204b514bb362713ccb bin/lldb-mesos-master.sh.in b1c7f9f1b98b5f410729f5a7e7a1729709f7e744 bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 bin/lldb-mesos-tests.sh.in f001b0b7f35839a101a86cd7df86fb7ebfc1c47e configure.ac 18bf4bfb345bdd443defccc4e53d357b35c7b533 docs/upgrades.md fe8b60470f7431accef44977e7036a2688289037 src/Makefile.am d58b46e99e0a041cf2a26abe44bbd1504a9539c0 src/log/catchup.cpp 4ee32f285f77eb2de661e22a301b743bb8a06f9c src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c src/slave/http.cpp c4f598faf6807214608cc89a6d9cf665133f95f3 src/webui/master/static/config.html PRE-CREATION src/webui/master/static/css/mesos.css 5b1227e9d64757f9fc106e497f7fa3ed72112c10 src/webui/master/static/directives/timestamp.html 5e422b9f22f8ddaf987feec3e02a849f21e5e22c src/webui/master/static/index.html f7f3d24abfee7d30691dbc2d7adf7c05c888a7b4 src/webui/master/static/js/app.js 4ccff6314c684ae4e917345fe41a95ccc0eb5803 src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94 Diff: https://reviews.apache.org/r/17431/diff/ Testing --- File Attachments Config Dialog https://reviews.apache.org/media/uploaded/files/2014/01/28/5499d3e5-077e-4aff-b29a-7d32134f29a0__Screenshot_2014-01-27_16.53.12.png Connection Issue Alert https://reviews.apache.org/media/uploaded/files/2014/01/28/dee8df12-0bae-48b5-a7ce-c07e0266c790__Screenshot_2014-01-28_12.44.53.png Thanks, Thomas Rampelberg
Re: Review Request 17082: Fixed display of table filter for empty tables
On Oct. 14, 2014, 9:38 a.m., Tobi Knaup wrote: Ship It! Awesome - will land this today. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17082/#review56538 --- On Jan. 17, 2014, 5:05 p.m., Thomas Rampelberg wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17082/ --- (Updated Jan. 17, 2014, 5:05 p.m.) Review request for mesos and Ross Allen. Repository: mesos-git Description --- Fixed display of table filter for empty tables Diffs - src/webui/master/static/directives/tableHeader.html 2acb6c8797e2aa8f8c2093f00ceaa52e22b497fd Diff: https://reviews.apache.org/r/17082/diff/ Testing --- Thanks, Thomas Rampelberg
Re: Review Request 21277: Passed CommandInfo to mesos-fetcher as JSON.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21277/#review56549 --- LGTM but needs rebase. Would be sweet to get in: much more robust than magic encoding :-) - Niklas Nielsen On May 9, 2014, 12:05 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21277/ --- (Updated May 9, 2014, 12:05 p.m.) Review request for mesos, Ben Mahler, Dominic Hamon, and Tom Arnfeld. Bugs: MESOS-1248 https://issues.apache.org/jira/browse/MESOS-1248 Repository: mesos-git Description --- See summary (and bug). Diffs - src/launcher/fetcher.cpp 8c9e20da8f39eb5e90403a5093cbea7fb2680468 src/slave/fetcher.hpp PRE-CREATION src/tests/fetcher_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/21277/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
On Oct. 10, 2014, 5:15 p.m., Dominic Hamon wrote: 3rdparty/libprocess/src/process.cpp, line 1631 https://reviews.apache.org/r/26583/diff/1/?file=717970#file717970line1631 this can't land until https://reviews.apache.org/r/26289/ which contains the configure.ac check for std::unique_ptr support. Dominic, what is the expected time frame for landing r26289? It contains more than just the check and we have a couple of patches that would depend on it. Can we pull out the check in a new patch to get going? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- On Oct. 10, 2014, 4:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 4:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26712: Add configuration check to libprocess for std::unique_ptr and std::move
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26712/#review56600 --- Ship it! Ship It! - Niklas Nielsen On Oct. 14, 2014, 4:01 p.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26712/ --- (Updated Oct. 14, 2014, 4:01 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- As summary Diffs - 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4 07b298f151094e818287f741b3e0efd28374e82b Diff: https://reviews.apache.org/r/26712/diff/ Testing --- make g++-4.4, g++-4.6, g++-4.8, clang++ Thanks, Dominic Hamon
Re: Review Request 26071: Sample isolator module.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26071/#review56742
---
src/examples/test_isolator_module.cpp
https://reviews.apache.org/r/26071/#comment97136
You wired up the modules - you can put in your own info (and serve the
purpose of tracking down the right author of broken modules ;-)
Here and below
src/examples/test_isolator_module.cpp
https://reviews.apache.org/r/26071/#comment97135
'Test' CPU Isolator module?
src/examples/test_isolator_module.cpp
https://reviews.apache.org/r/26071/#comment97137
Is this just to exercise the compatible logic? It always returns true.
Please comment if that is the purpose.
src/slave/containerizer/isolator.hpp
https://reviews.apache.org/r/26071/#comment97124
Can we move this to src/modules instead?
src/slave/containerizer/isolators/posix.hpp
https://reviews.apache.org/r/26071/#comment97131
Why this change? Here and below.
src/tests/module.hpp
https://reviews.apache.org/r/26071/#comment97125
Can you make the name and use of this module type more explicit? I had to
think twice, why this wasn't captured by the regular module 'kind'
src/tests/module.hpp
https://reviews.apache.org/r/26071/#comment97127
Can ModuleHelper and TestModule be merged?
src/tests/module.hpp
https://reviews.apache.org/r/26071/#comment97133
How about moving the statics into accessors instead? Here and below
src/tests/module.hpp
https://reviews.apache.org/r/26071/#comment97126
TestModule is a bit vague term for what it does (and a bit confusing mixed
with out previous notions of test/example module). I can't come up with a
better one on the spot, but let's think about it :-)
src/tests/module.cpp
https://reviews.apache.org/r/26071/#comment97130
So the idea is that moduleNames gets populated (later on) by flags? For
example ./mesos-tests --modules={} --module_tests={name: cpu_isolator,
value: org_apache_modules_FooBarIsolator} ?
Or is it only for internal mapping?
How about using strings to map instead of the enum? You would have to keep
a second map from the enum to the string eventually?
src/tests/module.cpp
https://reviews.apache.org/r/26071/#comment97122
How about using the libraryExtension constant as with the previous module
patch?
- Niklas Nielsen
On Oct. 15, 2014, 9:52 a.m., Kapil Arya wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26071/
---
(Updated Oct. 15, 2014, 9:52 a.m.)
Review request for mesos, Bernd Mathiske, Ian Downes, and Niklas Nielsen.
Bugs: MESOS-1384
https://issues.apache.org/jira/browse/MESOS-1384
Repository: mesos-git
Description
---
Created CPU and Memory isolation modules based on Posix CPU/Memory Isolators.
These modules are also hooked up to the relevant Isolator tests using the
typed test mechanism. It also paves the way for future integration with
custom modules specified on the command line.
Diffs
-
src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4
src/examples/test_isolator_module.cpp PRE-CREATION
src/module/manager.cpp 8cc79956a8d3d7cb27016889ec59d138a0b58e45
src/slave/containerizer/isolator.hpp
e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolators/posix.hpp
f120aafef96343d84f93c5636484509dc972a0a8
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
src/tests/module.hpp PRE-CREATION
src/tests/module.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26071/diff/
Testing
---
make check with an Isolator module test.
Thanks,
Kapil Arya
Re: Review Request 26698: Added StatusUpdateManager::unacknowledged() API call.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26698/#review56754 --- Think Adam caught most issues. Only two small nits/questions src/slave/status_update_manager.cpp https://reviews.apache.org/r/26698/#comment97141 Scan of s/ // per new C++11 style src/slave/status_update_manager.cpp https://reviews.apache.org/r/26698/#comment97142 Can you use stream-next() instead ? - Niklas Nielsen On Oct. 14, 2014, 11 a.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26698/ --- (Updated Oct. 14, 2014, 11 a.m.) Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen. Repository: mesos-git Description --- Added StatusUpdateManager::unacknowledged() API call so that slave can use this info during reregistration. Diffs - src/messages/messages.proto 8de7f9699f43aa2780f4a39fed087abcf5e5af99 src/slave/status_update_manager.hpp 24e3882ad1969c20a64daf90e408618c310e9a81 src/slave/status_update_manager.cpp 5d5cf234ef2dd47fa4b1f67be761dbca31659451 src/tests/status_update_manager_tests.cpp e9ef1e208cb01535e9366db7872b922c8f06ec40 Diff: https://reviews.apache.org/r/26698/diff/ Testing --- make check Ran new test 1000 times. Thanks, Vinod Kone
Re: Review Request 26699: Updated slave re-registration to send unacknowledged task states.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26699/#review56758 --- src/slave/slave.cpp https://reviews.apache.org/r/26699/#comment97147 What are your guarantees that task_id() is in the states map? Maybe guard it? - Niklas Nielsen On Oct. 14, 2014, 11:03 a.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26699/ --- (Updated Oct. 14, 2014, 11:03 a.m.) Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen. Bugs: MESOS-1799 and MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1799 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- Slave re-registration now sends both the latest state and unacknowledged state to the master. Diffs - src/slave/slave.hpp 342b09fc084c20d98d096bb129830440179c092c src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 src/tests/fault_tolerance_tests.cpp a75910d4f486230ba3f1d8927e5f1e5fda6e287b src/tests/slave_tests.cpp f585bdd20ae1af466f2c1b4d85331ac67451552f Diff: https://reviews.apache.org/r/26699/diff/ Testing --- make check Ran new test 1000 times. Thanks, Vinod Kone
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56800 --- Ship it! Ship It! - Niklas Nielsen On Oct. 12, 2014, 1:08 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 12, 2014, 1:08 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26476: Remove dynamic allocation from Option.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26476/#review56798 --- High-level: LGTM Have a couple of nits and we should be good to go. 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp https://reviews.apache.org/r/26476/#comment97221 As it's is a boolean expression, we wrap differently: http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Boolean_Expressions 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp https://reviews.apache.org/r/26476/#comment97219 t != NULL or isSome() 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp https://reviews.apache.org/r/26476/#comment97217 s/!t/t == NULL/ 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp https://reviews.apache.org/r/26476/#comment97218 s/t/t != NULL/ 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp https://reviews.apache.org/r/26476/#comment97216 reduce 2 spaces left - Niklas Nielsen On Oct. 8, 2014, 6:35 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26476/ --- (Updated Oct. 8, 2014, 6:35 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Remove dynamic allocations from Option class. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 47fe92c Diff: https://reviews.apache.org/r/26476/diff/ Testing --- make check support/mesos-style.py valgrind (reduced allocation count) Thanks, Joris Van Remoortere
Re: Review Request 26578: Memory cleanup: libprocess gc finalize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26578/#review56803 --- 3rdparty/libprocess/include/process/gc.hpp https://reviews.apache.org/r/26578/#comment97225 How about the include for unique ptr? 3rdparty/libprocess/include/process/gc.hpp https://reviews.apache.org/r/26578/#comment97227 Let 'iter' spell out what it is for (and expand to a full word). - Niklas Nielsen On Oct. 13, 2014, 10:29 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26578/ --- (Updated Oct. 13, 2014, 10:29 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Implement the finalize() virtual override for Libprocess' garbage collector. This terminates all processes registered with the garbage collector when the garbage collector terminates. Diffs - 3rdparty/libprocess/include/process/gc.hpp e83c636 Diff: https://reviews.apache.org/r/26578/diff/ Testing --- make check support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26674: Added name to Modules protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26674/#review56822 --- src/master/flags.hpp https://reviews.apache.org/r/26674/#comment97256 Please add a paragraph on, what happens when both file and name is set. src/module/manager.hpp https://reviews.apache.org/r/26674/#comment97255 Can you use the libraryExtension here (like in your previous patches?) src/tests/module_tests.cpp https://reviews.apache.org/r/26674/#comment97257 s/reset/Reset/ - Niklas Nielsen On Oct. 13, 2014, 6:24 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26674/ --- (Updated Oct. 13, 2014, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- To allow the users to specify a bare library names. For example, foo-2.3 will expand to libfoo-2.3.so on Linux and libfoo-2.3.dylib on OS X. Diffs - src/master/flags.hpp c41af1f4b1671d121eb617327086f5d80a2ba083 src/messages/messages.proto 8de7f9699f43aa2780f4a39fed087abcf5e5af99 src/module/manager.hpp 797728a8c8e88dd1a13142a355cbe0b1491fb7a2 src/module/manager.cpp 8cc79956a8d3d7cb27016889ec59d138a0b58e45 src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/tests/module_tests.cpp 71e7ef9ee9b21305528c00c7bfa2419ac9936974 Diff: https://reviews.apache.org/r/26674/diff/ Testing --- Added a new test and ran make check. Thanks, Kapil Arya
Re: Review Request 26629: Replace cerr/exit with EXIT when parsing master flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26629/#review56825 --- Ship it! Ship It! - Niklas Nielsen On Oct. 12, 2014, 1:11 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26629/ --- (Updated Oct. 12, 2014, 1:11 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Replace cerr/exit with EXIT when parsing master flags. Diffs - src/master/main.cpp 72e243a7374be36787a1e5a87d1ff370b8b9d4e0 Diff: https://reviews.apache.org/r/26629/diff/ Testing --- make check Thanks, Kapil Arya
