Re: Review Request 26423: Added documentation for egress rate limit control
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26423/#review58250 --- Patch looks great! Reviews applied: [26423] All tests passed. - Mesos ReviewBot On Oct. 23, 2014, 11:53 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26423/ --- (Updated Oct. 23, 2014, 11:53 p.m.) Review request for mesos, Ian Downes, Jie Yu, and Cong Wang. Repository: mesos-git Description --- see summary. Diffs - docs/network-monitoring.md 2a85f8b Diff: https://reviews.apache.org/r/26423/diff/ Testing --- not needed. Thanks, Chi Zhang
Re: Review Request 26622: Command Executor is broken when used with shell=false
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58254 --- Bad patch! Reviews applied: [26622] Failed command: ./support/apply-review.sh -n -r 26622 Error: --2014-10-24 08:20:08-- https://reviews.apache.org/r/26622/diff/raw/ Resolving reviews.apache.org (reviews.apache.org)... 140.211.11.74 Connecting to reviews.apache.org (reviews.apache.org)|140.211.11.74|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 6363 (6.2K) [text/x-patch] Saving to: '26622.patch' 0K ..100% 171M=0s 2014-10-24 08:20:09 (171 MB/s) - '26622.patch' saved [6363/6363] 'fullname' was not found 'email' was not found Successfully applied: Command Executor is broken when used with shell=false Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Review: https://reviews.apache.org/r/26622 fatal: empty ident name (for ) not allowed Failed to commit patch - Mesos ReviewBot On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 24, 2014, 2:04 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 27122: Reorganized authentication implementation locations.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27122/#review58253
---
Looks good. Just a couple of header guards and namespaces left to rename.
I assume you didn't actually change any code logic in this patch. Please tell
me if you did, and I will review those parts more closely.
src/authentication/cram_md5/authenticatee.hpp
https://reviews.apache.org/r/27122/#comment99211
New namespace?
src/authentication/cram_md5/authenticatee.hpp
https://reviews.apache.org/r/27122/#comment99212
s/2/1/ (Was a bug before, but is worth fixing here)
src/authentication/cram_md5/authenticator.hpp
https://reviews.apache.org/r/27122/#comment99213
s/SASL/CRAMMD5/
src/authentication/cram_md5/authenticator.hpp
https://reviews.apache.org/r/27122/#comment99214
namespace cram_md5 {?
src/authentication/cram_md5/auxprop.hpp
https://reviews.apache.org/r/27122/#comment99215
s/SASL/CRAMMD5/
src/authentication/cram_md5/auxprop.hpp
https://reviews.apache.org/r/27122/#comment99216
namespace cram_md5
src/authentication/cram_md5/auxprop.cpp
https://reviews.apache.org/r/27122/#comment99217
namespace cram_md5 {
src/tests/crammd5_authentication_tests.cpp
https://reviews.apache.org/r/27122/#comment99218
namespace cram_md5?
- Adam B
On Oct. 23, 2014, 6:32 p.m., Till Toenshoff wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27122/
---
(Updated Oct. 23, 2014, 6:32 p.m.)
Review request for mesos, Adam B and Vinod Kone.
Bugs: MESOS-1893
https://issues.apache.org/jira/browse/MESOS-1893
Repository: mesos-git
Description
---
For paving the way into further authentication mechanism implementations, the
existing is reorganized.
Move src/sasl to src/authentication/cram_md5, rename src/tests/sasl_tests.cpp
to src/tests/crammd5_authentication_tests.cpp.
Adapt depending include paths.
Rename specific test implementations from SASL to CRAMMD5Authentication.
Diffs
-
src/Makefile.am 2617f77
src/authentication/cram_md5/authenticatee.hpp PRE-CREATION
src/authentication/cram_md5/authenticator.hpp PRE-CREATION
src/authentication/cram_md5/auxprop.hpp PRE-CREATION
src/authentication/cram_md5/auxprop.cpp PRE-CREATION
src/master/master.cpp 95589b8
src/sasl/authenticatee.hpp ec2c841
src/sasl/authenticator.hpp 6f4d3db
src/sasl/auxprop.hpp 44e201b
src/sasl/auxprop.cpp 6de1222
src/sched/sched.cpp e89e5e5
src/scheduler/scheduler.cpp fb88a3e
src/slave/slave.cpp 55e5264
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
src/tests/sasl_tests.cpp 30556fa
Diff: https://reviews.apache.org/r/27122/diff/
Testing
---
make check
Thanks,
Till Toenshoff
Re: Review Request 27130: Libprocess Benchmark: require explicit run
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27130/#review58255 --- Patch looks great! Reviews applied: [27130] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 2:56 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27130/ --- (Updated Oct. 24, 2014, 2:56 a.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- require the libprocess benchmark to be explicitly run, as opposed to run by default during make check. Diffs - 3rdparty/libprocess/Makefile.am a9f6178 Diff: https://reviews.apache.org/r/27130/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 26860: Integrated CRAM-MD5 Authenticator module into tests.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26860/#review58256
---
Bad patch!
Reviews applied: [27122, 27122]
Failed command: ./support/apply-review.sh -n -r 27122
Error:
--2014-10-24 09:40:07-- https://reviews.apache.org/r/27122/diff/raw/
Resolving reviews.apache.org (reviews.apache.org)... 140.211.11.74
Connecting to reviews.apache.org (reviews.apache.org)|140.211.11.74|:443...
connected.
HTTP request sent, awaiting response... 200 OK
Length: 85581 (84K) [text/x-patch]
Saving to: '27122.patch'
0K .. .. .. .. .. 59% 523K 0s
50K .. .. .. ... 100% 1.03M=0.1s
2014-10-24 09:40:07 (656 KB/s) - '27122.patch' saved [85581/85581]
error: patch failed: src/Makefile.am:249
error: src/Makefile.am: patch does not apply
error: src/authentication/cram_md5/authenticatee.hpp: already exists in index
error: src/authentication/cram_md5/authenticator.hpp: already exists in index
error: src/authentication/cram_md5/auxprop.hpp: already exists in index
error: src/authentication/cram_md5/auxprop.cpp: already exists in index
error: patch failed: src/master/master.cpp:51
error: src/master/master.cpp: patch does not apply
error: src/sasl/authenticatee.hpp: does not exist in index
error: src/sasl/authenticator.hpp: does not exist in index
error: src/sasl/auxprop.hpp: does not exist in index
error: src/sasl/auxprop.cpp: does not exist in index
error: patch failed: src/sched/sched.cpp:59
error: src/sched/sched.cpp: patch does not apply
error: patch failed: src/scheduler/scheduler.cpp:53
error: src/scheduler/scheduler.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:62
error: src/slave/slave.cpp: patch does not apply
error: src/tests/crammd5_authentication_tests.cpp: already exists in index
error: src/tests/sasl_tests.cpp: does not exist in index
Failed to apply patch
- Mesos ReviewBot
On Oct. 24, 2014, 4:26 a.m., Till Toenshoff wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26860/
---
(Updated Oct. 24, 2014, 4:26 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
---
The CRAM-MD5 authenticator module is now usable via mesos-tests.sh
Example usage:
bin/mesos-tests.sh --authenticators=org_apache_mesos_authenticator_crammd5
--modules={\libraries\: [{\file\:
\src/.libs/libauthentication_crammd5.so\,\modules\: [{\name\:
\org_apache_mesos_authenticator_crammd5\}]}]}
Diffs
-
src/tests/flags.hpp 80f0cee
src/tests/mesos.cpp bff10d2
Diff: https://reviews.apache.org/r/26860/diff/
Testing
---
make check
bin/mesos-tests.sh --authenticators=org_apache_mesos_authenticator_crammd5
--modules={\libraries\: [{\file\:
\src/.libs/libauthentication_crammd5.dylib\,\modules\: [{\name\:
\org_apache_mesos_authenticator_crammd5\}]}]} --verbose
--gtest_filter=AuthenticationTest.AuthenticatedSlave
Shows:
I1024 06:16:14.764703 298631168 master.cpp:3874] Using
'org_apache_mesos_authenticator_crammd5' authenticator
bin/mesos-tests.sh --verbose
--gtest_filter=AuthenticationTest.AuthenticatedSlave
Shows:
I1024 06:21:39.526159 275554304 master.cpp:3865] Using default CRAM-MD5
authenticator
Thanks,
Till Toenshoff
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 24, 2014, 3:20 a.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [26622] Failed command: ./support/apply-review.sh -n -r 26622 Error: --2014-10-24 08:20:08-- https://reviews.apache.org/r/26622/diff/raw/ Resolving reviews.apache.org (reviews.apache.org)... 140.211.11.74 Connecting to reviews.apache.org (reviews.apache.org)|140.211.11.74|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 6363 (6.2K) [text/x-patch] Saving to: '26622.patch' 0K ..100% 171M=0s 2014-10-24 08:20:09 (171 MB/s) - '26622.patch' saved [6363/6363] 'fullname' was not found 'email' was not found Successfully applied: Command Executor is broken when used with shell=false Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Review: https://reviews.apache.org/r/26622 fatal: empty ident name (for ) not allowed Failed to commit patch What does: 'fullname' was not found 'email' was not found mean in this case? I didn't do anything different to generate the patch file than I did a week ago when I got a Patch looks great! reply. - R.B. --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58254 --- On Oct. 23, 2014, 9:04 p.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 23, 2014, 9:04 p.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 26622: Command Executor is broken when used with shell=false
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 --- src/tests/slave_tests.cpp https://reviews.apache.org/r/26622/#comment99235 I think I'll yank this out since this is already fixed. src/tests/slave_tests.cpp https://reviews.apache.org/r/26622/#comment99236 Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is going to fail on OSX. It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__ - Timothy Chen On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 24, 2014, 2:04 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits mixing mesos/libprocess/stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26825/#review58305 --- Ship it! Ship It! - Vinod Kone On Oct. 23, 2014, 5:17 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26825/ --- (Updated Oct. 23, 2014, 5:17 p.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-1712 and MESOS-1932 https://issues.apache.org/jira/browse/MESOS-1712 https://issues.apache.org/jira/browse/MESOS-1932 Repository: mesos-git Description --- Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess) Sample output: ``` ERROR: Commit spanning multiple projects Mesos rules state that a commit should only touch one mesos project (stout, libprocess, mesos). Paths grouped by project: mesos: baz libprocess: 3rdparty/libprocess/asdf stout: 3rdparty/libprocess/3rdparty/stout/foobar ``` Diffs - support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741 support/mesos_split.py PRE-CREATION Diff: https://reviews.apache.org/r/26825/diff/ Testing --- Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above. Thanks, Cody Maloney
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 24, 2014, 11:40 a.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 448 https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448 Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is going to fail on OSX. It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__ The part that actually breaks is any double-dash option arguments. If the OSX version of ls shares any other double-dash arg options with linux then just replace --author with that. - R.B. --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 --- On Oct. 23, 2014, 9:04 p.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 23, 2014, 9:04 p.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 23, 2014, 10:48 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 2558-2561 https://reviews.apache.org/r/26622/diff/2/?file=719277#file719277line2558 The comments here are confusing. We need to explictly call out that the reason we copying uris, environments and user is because we do not perform fetch/setting environments/change user in mesos-executor. We reply on containerizer to do that for mesos-executor so that the task can inherit those settings. R.B. Boyer wrote: Ah so mesos-executor requires only uris, environment, AND user? If that's the case I'm totally rewriting this to do explicit copying instead of playing protobuf tricks with merging. Jie Yu wrote: Rest of the fields are explicitly set in this function. I am not sure about 'container' because it's being deprecated. To be safe, let's copy 'container' as well. R.B. Boyer wrote: Will do: [uris, environment, user, container] Can you put the reason why we need to copy these fields (see my above comments) to the comment? - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58149 --- On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 24, 2014, 2:04 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Review Request 27150: Reordered functions in type_utils and added an equal comparator for Volume.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27150/ --- Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- I've reordered the functions (grouped by operator type, sorted by alphebet, moved non-trivial functions to cpp file). I haven't changed any content. Also, I added a equal comparator for Volume. Diffs - src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b src/common/type_utils.cpp 4f8d6ec912a41d11f16970ef7bbfa72bcbef8e8b Diff: https://reviews.apache.org/r/27150/diff/ Testing --- make check Thanks, Jie Yu
Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui #2481
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui/2481/changes
Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review58315 --- As discussed on IRC: -- please split this into multiple reviews (attributes methods and tests, compatible method, allocator changes, rest) -- fold slaveinfo_utils into protobuf_utils -- rethink reregisterSlave() (i'll help with this). one optimization i can think of which might help make reregisterSlave() simpler is to update the registrar to not do a log write if there is no mutation (e.g., readmit slave). It's fine if it seems too complicated and you want to punt on it too. - Vinod Kone On Oct. 22, 2014, 12:32 a.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 22, 2014, 12:32 a.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-1739 https://issues.apache.org/jira/browse/MESOS-1739 Repository: mesos-git Description --- Allows attributes and resources to be set to a superset of what they were previously on a slave restart. Incorporates all comments from: https://issues.apache.org/jira/browse/MESOS-1739 and the former review request: https://reviews.apache.org/r/25111/ Diffs - src/Makefile.am c44a9ad47d6e1262949b9049f4ae25b049440d99 src/common/attributes.hpp 0a043d5b5dca804c6dd215cabd2704f24df71a33 src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21 src/common/slaveinfo_utils.hpp PRE-CREATION src/common/slaveinfo_utils.cpp PRE-CREATION src/master/allocator.hpp 02d20d0cc802805bc702891306aa42894531b223 src/master/hierarchical_allocator_process.hpp 31dfb2cc86e2e74da43f05fbada10976ce65f3e4 src/master/master.hpp b1a2cd0f51f89d6dabbccaa67e0411fc55a4272f src/master/master.cpp e70cdbf78fd79738d38ecf314f9d8ee207558db2 src/slave/slave.cpp 55e5264a61ce43970e177cf0eb27dc8a2657e261 src/tests/attributes_tests.cpp 240a8cac18ac12178cf73e8eeb88bd50e3fcc03b src/tests/mesos.hpp e36e13843a36360d69a01f62356370bea588a337 src/tests/slave_tests.cpp a1bd00cffe9de178b0b188c3556e479cd1f6d566 Diff: https://reviews.apache.org/r/25525/diff/ Testing --- make check on localhost Thanks, Cody Maloney
Re: Review Request 27102: Moved framework id validation from scheduler driver to master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27102/#review58320 --- Ship it! src/master/master.cpp https://reviews.apache.org/r/27102/#comment99275 Can you update the existing ExecutorInfo checker instead? We already check presence there, but not equality. - Ben Mahler On Oct. 23, 2014, 11:32 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27102/ --- (Updated Oct. 23, 2014, 11:32 p.m.) Review request for mesos, Ben Mahler and Dominic Hamon. Bugs: MESOS-1972 https://issues.apache.org/jira/browse/MESOS-1972 Repository: mesos-git Description --- Moved framework id validation from scheduler driver to master. Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver. Diffs - src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98 Diff: https://reviews.apache.org/r/27102/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 24, 2014, 4:40 p.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 448 https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448 Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is going to fail on OSX. It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__ R.B. Boyer wrote: The part that actually breaks is any double-dash option arguments. If the OSX version of ls shares any other double-dash arg options with linux then just replace --author with that. unfortunately ls doesn't have any double dash options, and I just looked at /usr/bin and none of the non-intrusive binaries has any double dash options too. I can however do a echo --author, that would have broke earlier right? - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 --- On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 24, 2014, 2:04 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27150: Reordered functions in type_utils and added an equal comparator for Volume.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27150/#review58328 --- Patch looks great! Reviews applied: [27150] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 6:22 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27150/ --- (Updated Oct. 24, 2014, 6:22 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- I've reordered the functions (grouped by operator type, sorted by alphebet, moved non-trivial functions to cpp file). I haven't changed any content. Also, I added a equal comparator for Volume. Diffs - src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b src/common/type_utils.cpp 4f8d6ec912a41d11f16970ef7bbfa72bcbef8e8b Diff: https://reviews.apache.org/r/27150/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 27150: Reordered functions in type_utils and added an equal comparator for Volume.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27150/#review58329 --- Ship it! src/common/type_utils.hpp https://reviews.apache.org/r/27150/#comment99281 Why did you wrap all the ones above but not this one? - Ben Mahler On Oct. 24, 2014, 6:22 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27150/ --- (Updated Oct. 24, 2014, 6:22 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- I've reordered the functions (grouped by operator type, sorted by alphebet, moved non-trivial functions to cpp file). I haven't changed any content. Also, I added a equal comparator for Volume. Diffs - src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b src/common/type_utils.cpp 4f8d6ec912a41d11f16970ef7bbfa72bcbef8e8b Diff: https://reviews.apache.org/r/27150/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/#review58330 --- Ship it! Thanks Cody! Just some trivial comments below and we can get this committed! configure.ac https://reviews.apache.org/r/26426/#comment99282 Can you wrap after the comma to be consistent with the other AC_ARG_ENABLE blocks? Any reason to do it differently here? configure.ac https://reviews.apache.org/r/26426/#comment99283 How about s/$debug/$debug_flags/ and s/$optimize/$optimize_flags/ ? - Ben Mahler On Oct. 23, 2014, 9:20 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/ --- (Updated Oct. 23, 2014, 9:20 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair. Repository: mesos-git Description --- Reworks buiding mesos in a debug vs. a release configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity None: '-O0 -g1' --enable-optimize == '-O2' --enable-debug == '-g' --enable-optimize --enable-debug == '-O2 -g' If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use). Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion. Diffs - configure.ac 2b372e06006250b5230956ef096473e98f3fa590 Diff: https://reviews.apache.org/r/26426/diff/ Testing --- Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified Thanks, Cody Maloney
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/#review58332 --- Ship it! src/slave/containerizer/isolators/network/port_mapping.cpp https://reviews.apache.org/r/27154/#comment99284 Add a Note: prefix. It will be great if you can add an ASCII chart to show how veth and eth0 are connected. - Jie Yu On Oct. 24, 2014, 7:08 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 7:08 p.m.) Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Jenkins build is back to normal : Mesos-Ubuntu-distcheck #434
See https://builds.apache.org/job/Mesos-Ubuntu-distcheck/434/changes
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/#review58349 --- Patch looks great! Reviews applied: [27154] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 7:08 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 7:08 p.m.) Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27113: Libprocess benchmark cleanup
On Oct. 23, 2014, 10:18 p.m., Ben Mahler wrote: 3rdparty/libprocess/src/tests/benchmarks.cpp, line 149 https://reviews.apache.org/r/27113/diff/1/?file=731328#file731328line149 Why is this a set, don't you only need to know whether you're linked? (i.e. boolean). This is just 1:1 client:server in terms of messaging, right? It's a bit confusing as to why there is 1 'other', but many 'linkedPorts'.. There are many clients connecting to a single server. The server keeps track of which clients it has linked with to ensure that it links back to all of them. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27113/#review58124 --- On Oct. 24, 2014, 3:09 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27113/ --- (Updated Oct. 24, 2014, 3:09 a.m.) Review request for mesos, Ben Mahler and Niklas Nielsen. Bugs: MESOS-1980 https://issues.apache.org/jira/browse/MESOS-1980 Repository: mesos-git Description --- Add a comment to BenchmarkProcess. Remove setLink() as it is not strictly necessary. Diffs - 3rdparty/libprocess/src/tests/benchmarks.cpp 79a650b Diff: https://reviews.apache.org/r/27113/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27113: Libprocess benchmark cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27113/ --- (Updated Oct. 24, 2014, 8:46 p.m.) Review request for mesos, Ben Mahler and Niklas Nielsen. Changes --- address bmahler's issues. Bugs: MESOS-1980 https://issues.apache.org/jira/browse/MESOS-1980 Repository: mesos-git Description --- Add a comment to BenchmarkProcess. Remove setLink() as it is not strictly necessary. Diffs (updated) - 3rdparty/libprocess/src/tests/benchmarks.cpp 3177a8e Diff: https://reviews.apache.org/r/27113/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 24, 2014, 11:40 a.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 448 https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448 Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is going to fail on OSX. It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__ R.B. Boyer wrote: The part that actually breaks is any double-dash option arguments. If the OSX version of ls shares any other double-dash arg options with linux then just replace --author with that. Timothy Chen wrote: unfortunately ls doesn't have any double dash options, and I just looked at /usr/bin and none of the non-intrusive binaries has any double dash options too. I can however do a echo --author, that would have broke earlier right? I don't have access to an OSX box, nor will I have time until mid next week to revisit this ticket. If you can, please try swapping out [echo, --author] for my ls example, reverting slave.cpp, and then: GTEST_FILTER=SlaveTest.MesosExecutorCommandTaskWithArgsList make check To just double check that echo --author killed mesos-executor on OSX with the unpatched protobuf stuff. - R.B. --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 --- On Oct. 23, 2014, 9:04 p.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 23, 2014, 9:04 p.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/ --- (Updated Oct. 24, 2014, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair. Changes --- Update per Dominic Hamon and Ben Mahler's comments. Repository: mesos-git Description --- Reworks buiding mesos in a debug vs. a release configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity None: '-O0 -g1' --enable-optimize == '-O2' --enable-debug == '-g' --enable-optimize --enable-debug == '-O2 -g' If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use). Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion. Diffs (updated) - configure.ac 2b372e06006250b5230956ef096473e98f3fa590 Diff: https://reviews.apache.org/r/26426/diff/ Testing --- Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified Thanks, Cody Maloney
Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos
On Oct. 23, 2014, 9:50 p.m., Dominic Hamon wrote: configure.ac, line 289 https://reviews.apache.org/r/26426/diff/3/?file=731214#file731214line289 might be simpler to set a default then override: debug=-g1 if ... debug=-g else if ... debug= fi optimize=-O0 if ... optimize=-O2 fi Updated for the debug case where there is a defualt then a in other cases do this, in other cases the default should be different. For optimize I like that I see I will definitively get either -O0 or -O2 based on which way the if goes. - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/#review58118 --- On Oct. 24, 2014, 9:29 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/ --- (Updated Oct. 24, 2014, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair. Repository: mesos-git Description --- Reworks buiding mesos in a debug vs. a release configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity None: '-O0 -g1' --enable-optimize == '-O2' --enable-debug == '-g' --enable-optimize --enable-debug == '-O2 -g' If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use). Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion. Diffs - configure.ac 2b372e06006250b5230956ef096473e98f3fa590 Diff: https://reviews.apache.org/r/26426/diff/ Testing --- Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified Thanks, Cody Maloney
Re: Review Request 26622: Command Executor is broken when used with shell=false
On Oct. 24, 2014, 4:40 p.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 448 https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448 Just tried bin/ls on OSX and realize Mac's ls has no --author flag so this is going to fail on OSX. It's ok if we don't support it but then we need to wrap the test in #ifdef __linux__ R.B. Boyer wrote: The part that actually breaks is any double-dash option arguments. If the OSX version of ls shares any other double-dash arg options with linux then just replace --author with that. Timothy Chen wrote: unfortunately ls doesn't have any double dash options, and I just looked at /usr/bin and none of the non-intrusive binaries has any double dash options too. I can however do a echo --author, that would have broke earlier right? R.B. Boyer wrote: I don't have access to an OSX box, nor will I have time until mid next week to revisit this ticket. If you can, please try swapping out [echo, --author] for my ls example, reverting slave.cpp, and then: GTEST_FILTER=SlaveTest.MesosExecutorCommandTaskWithArgsList make check To just double check that echo --author killed mesos-executor on OSX with the unpatched protobuf stuff. Ok I'll do that. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 --- On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/ --- (Updated Oct. 24, 2014, 2:04 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. Bugs: MESOS-1873 https://issues.apache.org/jira/browse/MESOS-1873 Repository: mesos-git Description --- Basically if you use shell=false with a non-empty argument list and the Command Executor it is completely broken. When we clone the env vars to fork mesos-executor all of the original cmd args are cloned as well (unintentionally) due to some protocol-buffer merge shenanigans. Don't pass task-related arguments to mesos-executor. The description on the linked jira ticket goes into more detail. Diffs - src/slave/slave.cpp 55e5264 src/tests/slave_tests.cpp a1bd00c Diff: https://reviews.apache.org/r/26622/diff/ Testing --- make check added new test SlaveTest.GetExecutorInfo verifying the explicit desired behavior. Thanks, R.B. Boyer
Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/ --- (Updated Oct. 24, 2014, 9:35 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair. Bugs: MESOS-1985 https://issues.apache.org/jira/browse/MESOS-1985 Repository: mesos-git Description --- Reworks buiding mesos in a debug vs. a release configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity None: '-O0 -g1' --enable-optimize == '-O2' --enable-debug == '-g' --enable-optimize --enable-debug == '-O2 -g' If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use). Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion. Diffs - configure.ac 2b372e06006250b5230956ef096473e98f3fa590 Diff: https://reviews.apache.org/r/26426/diff/ Testing --- Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified Thanks, Cody Maloney
Re: Review Request 27113: Libprocess benchmark cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27113/#review58386 --- Patch looks great! Reviews applied: [25448, 27113] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 8:46 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27113/ --- (Updated Oct. 24, 2014, 8:46 p.m.) Review request for mesos, Ben Mahler and Niklas Nielsen. Bugs: MESOS-1980 https://issues.apache.org/jira/browse/MESOS-1980 Repository: mesos-git Description --- Add a comment to BenchmarkProcess. Remove setLink() as it is not strictly necessary. Diffs - 3rdparty/libprocess/src/tests/benchmarks.cpp 3177a8e Diff: https://reviews.apache.org/r/27113/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 9:48 p.m.) Review request for mesos, Jie Yu and Cong Wang. Changes --- addressed comments. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 9:49 p.m.) Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 9:49 p.m.) Review request for mesos, Jie Yu and Cong Wang. Changes --- addressed comments. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27130: Libprocess Benchmark: require explicit run
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27130/#review58394 --- Ship it! Ship It! - Ben Mahler On Oct. 24, 2014, 2:56 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27130/ --- (Updated Oct. 24, 2014, 2:56 a.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- require the libprocess benchmark to be explicitly run, as opposed to run by default during make check. Diffs - 3rdparty/libprocess/Makefile.am a9f6178 Diff: https://reviews.apache.org/r/27130/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 27127: Add getns() for namespaces and killall() for pid namespaces.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/ --- (Updated Oct. 24, 2014, 2:53 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Changes --- Split as per review comment. Repository: mesos-git Description --- getns() to get the namespace inode. killall() to kill all processes in a pid namespace. Diffs (updated) - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27127/diff/ Testing --- make check # new test added Thanks, Ian Downes
Re: Review Request 27091: Move Linux namespace functions into linux/.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27091/ --- (Updated Oct. 24, 2014, 2:53 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Changes --- Removed erroneous includes, included missing file. Deferred other cleanup to a subsequent review. Repository: mesos-git Description --- A following commit will remove this code from stout. Diffs (updated) - src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09 src/linux/ns.hpp PRE-CREATION src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bca08de55f19bfff7b8828dfa4fb6c51f30 src/tests/ns_tests.cpp PRE-CREATION src/tests/setns_test_helper.hpp PRE-CREATION src/tests/setns_test_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27091/diff/ Testing --- make check Thanks, Ian Downes
Re: Review Request 25966: Use pid namespace in LinuxLauncher::destroy().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25966/ --- (Updated Oct. 24, 2014, 2:53 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- Check if a container is running in a pid namespace and thus all processes can be killed by the kernel, rather than using the freezer. Diffs (updated) - src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/tests/slave_recovery_tests.cpp 813e2d66f0127a84282d3b4ed0423eab36e7ef8b Diff: https://reviews.apache.org/r/25966/diff/ Testing --- Thanks, Ian Downes
Review Request 27165: Add ns::pid::destroy() to destroy a pid namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/ --- Review request for mesos and Ben Mahler. Repository: mesos-git Description --- All processes are signalled with SIGKILL then reaped. The order of signalling is not determined, i.e., generally the init pid is not the first pid signalled. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27165/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 27127: Add getns() for namespaces.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/ --- (Updated Oct. 24, 2014, 2:54 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Summary (updated) - Add getns() for namespaces. Repository: mesos-git Description (updated) --- getns() to get the namespace inode. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27127/diff/ Testing --- make check # new test added Thanks, Ian Downes
Re: Review Request 27165: Add ns::pid::destroy() to destroy a pid namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/ --- (Updated Oct. 24, 2014, 2:54 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- All processes are signalled with SIGKILL then reaped. The order of signalling is not determined, i.e., generally the init pid is not the first pid signalled. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27165/diff/ Testing (updated) --- make check # new test added Thanks, Ian Downes
Review Request 27164: Include changes to isolation flag when creating Mesos containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27164/ --- Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Include changes to isolation flag when creating Mesos containerizer. Diffs - src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 Diff: https://reviews.apache.org/r/27164/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/#review58397 --- Ship it! Ship It! - Ian Downes On Oct. 24, 2014, 2:49 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 2:49 p.m.) Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/ --- (Updated Oct. 24, 2014, 3:04 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace. Diffs (updated) - src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09 src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291 Diff: https://reviews.apache.org/r/25865/diff/ Testing --- Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc). Thanks, Ian Downes
Re: Review Request 27154: PortMappingIsolator: Swap TX/RX in statistic collection in usage
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/#review58408 --- Patch looks great! Reviews applied: [27154] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 9:49 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27154/ --- (Updated Oct. 24, 2014, 9:49 p.m.) Review request for mesos, Jie Yu and Cong Wang. Bugs: mesos-1989 https://issues.apache.org/jira/browse/mesos-1989 Repository: mesos-git Description --- statistics collected on the host end of veth pair have reversed RX/TX of those in the container. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bc Diff: https://reviews.apache.org/r/27154/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 27127: Add getns() for namespaces.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/#review58411 --- src/linux/ns.hpp https://reviews.apache.org/r/27127/#comment99405 Should this be wrapped in Error() too? - Timothy Chen On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- getns() to get the namespace inode. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27127/diff/ Testing --- make check # new test added Thanks, Ian Downes
Review Request 27175: Added support for handling 'rc' tag to Version.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27175/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1987 https://issues.apache.org/jira/browse/MESOS-1987 Repository: mesos-git Description --- Now it can parse strings like 1.2.3-rc4. Other tags are still discarded. Updated os::release() to use Version::parse() instead of sscanf. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5d3cbba289d8f9d57c2fd49dd6f72225bb2fb8c2 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp 090fcf09dd96538a8748cf4443d150911e2c0d27 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp e8f8358f3c113b4e21e30cb5e9d6a3d229191484 Diff: https://reviews.apache.org/r/27175/diff/ Testing --- Enhanced version tests and ran make check. Thanks, Kapil Arya
Re: Review Request 25966: Use pid namespace in LinuxLauncher::destroy().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25966/#review58425 --- Bad patch! Reviews applied: [27091, 27127, 27091] Failed command: ./support/apply-review.sh -n -r 27091 Error: 2014-10-24 22:56:52 URL:https://reviews.apache.org/r/27091/diff/raw/ [16720/16720] - 27091.patch [1] error: patch failed: src/Makefile.am:406 error: src/Makefile.am: patch does not apply error: src/linux/ns.hpp: already exists in index error: patch failed: src/slave/containerizer/isolators/network/port_mapping.cpp:45 error: src/slave/containerizer/isolators/network/port_mapping.cpp: patch does not apply error: src/tests/ns_tests.cpp: already exists in index error: src/tests/setns_test_helper.hpp: already exists in index error: src/tests/setns_test_helper.cpp: already exists in index Failed to apply patch - Mesos ReviewBot On Oct. 24, 2014, 9:53 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25966/ --- (Updated Oct. 24, 2014, 9:53 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- Check if a container is running in a pid namespace and thus all processes can be killed by the kernel, rather than using the freezer. Diffs - src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/tests/slave_recovery_tests.cpp 813e2d66f0127a84282d3b4ed0423eab36e7ef8b Diff: https://reviews.apache.org/r/25966/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 27165: Add ns::pid::destroy() to destroy a pid namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/#review58430 --- Bad patch! Reviews applied: [27091, 27091] Failed command: ./support/apply-review.sh -n -r 27091 Error: 2014-10-24 23:11:12 URL:https://reviews.apache.org/r/27091/diff/raw/ [16720/16720] - 27091.patch [1] error: patch failed: src/Makefile.am:406 error: src/Makefile.am: patch does not apply error: src/linux/ns.hpp: already exists in index error: patch failed: src/slave/containerizer/isolators/network/port_mapping.cpp:45 error: src/slave/containerizer/isolators/network/port_mapping.cpp: patch does not apply error: src/tests/ns_tests.cpp: already exists in index error: src/tests/setns_test_helper.hpp: already exists in index error: src/tests/setns_test_helper.cpp: already exists in index Failed to apply patch - Mesos ReviewBot On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- All processes are signalled with SIGKILL then reaped. The order of signalling is not determined, i.e., generally the init pid is not the first pid signalled. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27165/diff/ Testing --- make check # new test added Thanks, Ian Downes
Re: Review Request 27127: Add getns() for namespaces.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/#review58428 --- Ship it! src/linux/ns.hpp https://reviews.apache.org/r/27127/#comment99424 To avoid the race where the process exits right before this, can you check for support based on init or self? src/linux/ns.hpp https://reviews.apache.org/r/27127/#comment99426 s/buf/s/ src/tests/ns_tests.cpp https://reviews.apache.org/r/27127/#comment99432 We can use the ROOT_ filter for this and the other tests. src/tests/ns_tests.cpp https://reviews.apache.org/r/27127/#comment99441 Ditto on !empty - Ben Mahler On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27127/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- getns() to get the namespace inode. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27127/diff/ Testing --- make check # new test added Thanks, Ian Downes
Re: Review Request 27165: Add ns::pid::destroy() to destroy a pid namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/#review58441 --- Ship it! src/linux/ns.hpp https://reviews.apache.org/r/27165/#comment99446 Can you hide this in an internal namespace? This can just return Nothing directly. src/linux/ns.hpp https://reviews.apache.org/r/27165/#comment99451 Can you move this above the call to getns? src/tests/ns_tests.cpp https://reviews.apache.org/r/27165/#comment99459 children src/tests/ns_tests.cpp https://reviews.apache.org/r/27165/#comment99461 Can we use ROOT? - Ben Mahler On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27165/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- All processes are signalled with SIGKILL then reaped. The order of signalling is not determined, i.e., generally the init pid is not the first pid signalled. Diffs - src/linux/ns.hpp PRE-CREATION src/tests/ns_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/27165/diff/ Testing --- make check # new test added Thanks, Ian Downes
Re: Review Request 27166: Correctly recover pid in Linux launcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27166/#review58446 --- Ship it! src/slave/containerizer/linux_launcher.cpp https://reviews.apache.org/r/27166/#comment99463 Should we have a small note similar to your review description? - Ben Mahler On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27166/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- If the freezer cgroup is absent (if the slave terminates after the cgroup is destroyed but before realizing) we should still recover the pid because we check this on destroy(). Diffs - src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b Diff: https://reviews.apache.org/r/27166/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 27164: Include changes to isolation flag when creating Mesos containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27164/#review58447 --- Ship it! src/slave/containerizer/mesos/containerizer.cpp https://reviews.apache.org/r/27164/#comment99464 Can you just take a non-const copy in the argument list? - Ben Mahler On Oct. 24, 2014, 9:55 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27164/ --- (Updated Oct. 24, 2014, 9:55 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Include changes to isolation flag when creating Mesos containerizer. Diffs - src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 Diff: https://reviews.apache.org/r/27164/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 25966: Use pid namespace in LinuxLauncher::destroy().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25966/#review58448 --- Ship it! src/tests/slave_recovery_tests.cpp https://reviews.apache.org/r/25966/#comment99465 registerExecutorMessage Ditto below. - Ben Mahler On Oct. 24, 2014, 9:53 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25966/ --- (Updated Oct. 24, 2014, 9:53 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- Check if a container is running in a pid namespace and thus all processes can be killed by the kernel, rather than using the freezer. Diffs - src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/tests/slave_recovery_tests.cpp 813e2d66f0127a84282d3b4ed0423eab36e7ef8b Diff: https://reviews.apache.org/r/25966/diff/ Testing --- Thanks, Ian Downes
Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2207
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME/2207/changes Changes: [tstclair] Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos -- [...truncated 64167 lines...] I1025 00:03:29.005030 6786 slave.cpp:2522] Received ping from slave-observer(1)@67.195.81.187:42414 I1025 00:03:29.174675 6797 monitor.cpp:140] Failed to collect resource usage for container '6ea903e2-87d0-45a5-8ad1-69924f040402' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: 6ea903e2-87d0-45a5-8ad1-69924f040402 I1025 00:03:29.176897 6787 monitor.cpp:140] Failed to collect resource usage for container 'd19ca0b7-5dd9-4a84-9e92-b72f3b3853ae' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: d19ca0b7-5dd9-4a84-9e92-b72f3b3853ae I1025 00:03:29.485119 6789 slave.cpp:2522] Received ping from slave-observer(2)@67.195.81.187:42414 I1025 00:03:29.618518 6788 hierarchical_allocator_process.hpp:734] Offering cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141024-233658-3142697795-42414-6774-S0 to framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.618857 6788 hierarchical_allocator_process.hpp:816] Filtered mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):2 on slave 20141024-233658-3142697795-42414-6774-S1 for framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.618991 6788 hierarchical_allocator_process.hpp:734] Offering mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1 on slave 20141024-233658-3142697795-42414-6774-S2 to framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.619336 6788 hierarchical_allocator_process.hpp:659] Performed allocation for 3 slaves in 1.120275ms I1025 00:03:29.619598 6787 master.cpp:3795] Sending 2 offers to framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.621063 6788 sched.cpp:544] Scheduler::resourceOffers took 1.235468ms I1025 00:03:29.621443 6788 master.cpp:2321] Processing reply for offers: [ 20141024-233658-3142697795-42414-6774-O952 ] on slave 20141024-233658-3142697795-42414-6774-S2 at slave(3)@67.195.81.187:42414 (pomona.apache.org) for framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.621809 6788 master.cpp:2321] Processing reply for offers: [ 20141024-233658-3142697795-42414-6774-O953 ] on slave 20141024-233658-3142697795-42414-6774-S0 at slave(2)@67.195.81.187:42414 (pomona.apache.org) for framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.622165 6787 hierarchical_allocator_process.hpp:563] Recovered mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1 (total allocatable: mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1) on slave 20141024-233658-3142697795-42414-6774-S2 from framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.622200 6787 hierarchical_allocator_process.hpp:599] Framework 20141024-233658-3142697795-42414-6774- filtered slave 20141024-233658-3142697795-42414-6774-S2 for 5secs I1025 00:03:29.622339 6787 hierarchical_allocator_process.hpp:563] Recovered cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] (total allocatable: cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]) on slave 20141024-233658-3142697795-42414-6774-S0 from framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.622371 6787 hierarchical_allocator_process.hpp:599] Framework 20141024-233658-3142697795-42414-6774- filtered slave 20141024-233658-3142697795-42414-6774-S0 for 5secs I1025 00:03:29.681506 6797 slave.cpp:2522] Received ping from slave-observer(3)@67.195.81.187:42414 I1025 00:03:30.175997 6795 monitor.cpp:140] Failed to collect resource usage for container '6ea903e2-87d0-45a5-8ad1-69924f040402' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: 6ea903e2-87d0-45a5-8ad1-69924f040402 I1025 00:03:30.178053 6786 monitor.cpp:140] Failed to collect resource usage for container 'd19ca0b7-5dd9-4a84-9e92-b72f3b3853ae' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: d19ca0b7-5dd9-4a84-9e92-b72f3b3853ae 2014-10-25 00:03:30,187:27064(0x2adb076bf700):ZOO_ERROR@handle_socket_error_msg@1697: Socket [127.0.0.1:32947] zk retcode=-4, errno=111(Connection refused): server refused to accept the client I1025 00:03:30.619998 6790 hierarchical_allocator_process.hpp:816] Filtered mem
Re: Review Request 27166: Correctly recover pid in Linux launcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27166/#review58452 --- Patch looks great! Reviews applied: [27166] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 9:54 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27166/ --- (Updated Oct. 24, 2014, 9:54 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- If the freezer cgroup is absent (if the slave terminates after the cgroup is destroyed but before realizing) we should still recover the pid because we check this on destroy(). Diffs - src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b Diff: https://reviews.apache.org/r/27166/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/#review58450 --- Ship it! src/slave/containerizer/isolators/namespaces/pid.hpp https://reviews.apache.org/r/25865/#comment99467 s/.././ src/slave/containerizer/isolators/namespaces/pid.cpp https://reviews.apache.org/r/25865/#comment99469 Can you wrap at 70 everywhere for comments? src/slave/containerizer/isolators/namespaces/pid.cpp https://reviews.apache.org/r/25865/#comment99471 Maybe a bit of an explanation of the masking technique used? src/slave/containerizer/isolators/namespaces/pid.cpp https://reviews.apache.org/r/25865/#comment99479 I'm curious if we can name bindSource and bindTarget to reflect what these are returning without referring to the way the caller uses them: source - namespaceProcfile? target - namespaceHandle? src/slave/containerizer/isolators/namespaces/pid.cpp https://reviews.apache.org/r/25865/#comment99478 Maybe add a string about stat failing inside the error message? - Ben Mahler On Oct. 24, 2014, 10:04 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/ --- (Updated Oct. 24, 2014, 10:04 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace. Diffs - src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09 src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291 Diff: https://reviews.apache.org/r/25865/diff/ Testing --- Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc). Thanks, Ian Downes
Re: Review Request 27175: Added support for handling 'rc' tag to Version.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27175/#review58472 --- Patch looks great! Reviews applied: [27175] All tests passed. - Mesos ReviewBot On Oct. 24, 2014, 10:55 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27175/ --- (Updated Oct. 24, 2014, 10:55 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1987 https://issues.apache.org/jira/browse/MESOS-1987 Repository: mesos-git Description --- Now it can parse strings like 1.2.3-rc4. Other tags are still discarded. Updated os::release() to use Version::parse() instead of sscanf. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5d3cbba289d8f9d57c2fd49dd6f72225bb2fb8c2 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp 090fcf09dd96538a8748cf4443d150911e2c0d27 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp e8f8358f3c113b4e21e30cb5e9d6a3d229191484 Diff: https://reviews.apache.org/r/27175/diff/ Testing --- Enhanced version tests and ran make check. Thanks, Kapil Arya
Re: Review Request 27111: Block IO Isolator: usage metrics
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27111/#review58418
---
Close to ready. Major points are cleaning up the cgroups statistics functions
and making the test child safe.
src/linux/cgroups.hpp
https://reviews.apache.org/r/27111/#comment99413
s/The a pair/The pair/
src/linux/cgroups.cpp
https://reviews.apache.org/r/27111/#comment99414
We accept now.
src/linux/cgroups.cpp
https://reviews.apache.org/r/27111/#comment99481
I think it's error-prone to have the implicit read, write pairing and
specifying the file.
What about hiding this function in an internal namespace and exposing the
various statistics explicitly under a descriptive namespace, e.g., perhaps
something like this:
Bytes blkio::statistics::bytes::{read,write}()
uint64_t blkio::statistics::iops::{read,write}()
Yes, converting to Bytes to return is unnecessary in your usage but it
avoids any ambiguity for the user of this library function.
src/linux/fs.cpp
https://reviews.apache.org/r/27111/#comment99419
Any tests for this?
src/linux/fs.cpp
https://reviews.apache.org/r/27111/#comment99417
move this to after the file.fail() check?
src/slave/containerizer/isolators/cgroups/blkio.cpp
https://reviews.apache.org/r/27111/#comment99473
kill newline
src/slave/containerizer/isolators/cgroups/blkio.cpp
https://reviews.apache.org/r/27111/#comment99482
What's the difference between blkio.throttle.io_service_bytes and
blkio.io_service_bytes?
src/tests/isolator_tests.cpp
https://reviews.apache.org/r/27111/#comment99490
s/alligned/aligned/
src/tests/isolator_tests.cpp
https://reviews.apache.org/r/27111/#comment99492
posix_memalign is definitely not async-signal-safe
src/tests/isolator_tests.cpp
https://reviews.apache.org/r/27111/#comment99488
s/ret/write/? or count or ...
something other than 'ret', please :-)
pwrite() is not listed as async-signal-safe:
http://man7.org/linux/man-pages/man7/signal.7.html
src/tests/isolator_tests.cpp
https://reviews.apache.org/r/27111/#comment99489
ditto
src/tests/isolator_tests.cpp
https://reviews.apache.org/r/27111/#comment99484
This setup function must be async-signal-safe as it's called between the
fork and exec, and we're forking from a threaded parent.
http://man7.org/linux/man-pages/man7/signal.7.html
Can you verify that everything in there is safe (and comment that it is) or
put this code into a separate helper binary?
- Ian Downes
On Oct. 23, 2014, 2:58 p.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27111/
---
(Updated Oct. 23, 2014, 2:58 p.m.)
Review request for mesos, Benjamin Hindman and Ian Downes.
Bugs: MESOS-1977
https://issues.apache.org/jira/browse/MESOS-1977
Repository: mesos-git
Description
---
Implement a block io isolator that just publishes read / write, bytes /
operations per second.
A split of r25922 as suggested by Ian.
Diffs
-
include/mesos/mesos.proto 6b93e90
src/Makefile.am 2617f77
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 62df4b7
src/linux/fs.hpp ac8b5f4
src/linux/fs.cpp b01d14c
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 9d08329
src/tests/isolator_tests.cpp 52b38a3
Diff: https://reviews.apache.org/r/27111/diff/
Testing
---
make check
sudo ./mesos-tests --gtest_filter=BlkIOIsolatorTest*
Thanks,
Joris Van Remoortere
Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2208
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME/2208/changes
Re: Review Request 27164: Include changes to isolation flag when creating Mesos containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27164/#review58475 --- Bad patch! Reviews applied: [27164] Failed command: ./support/apply-review.sh -n -r 27164 Error: 2014-10-25 01:24:25 URL:https://reviews.apache.org/r/27164/diff/raw/ [1919/1919] - 27164.patch [1] error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:145 error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Oct. 24, 2014, 9:55 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27164/ --- (Updated Oct. 24, 2014, 9:55 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Include changes to isolation flag when creating Mesos containerizer. Diffs - src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 Diff: https://reviews.apache.org/r/27164/diff/ Testing --- Thanks, Ian Downes
Re: Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2207
filed: https://issues.apache.org/jira/browse/MESOS-1994 On Fri, Oct 24, 2014 at 5:03 PM, Apache Jenkins Server jenk...@builds.apache.org wrote: See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME/2207/changes Changes: [tstclair] Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos -- [...truncated 64167 lines...] I1025 00:03:29.005030 6786 slave.cpp:2522] Received ping from slave-observer(1)@67.195.81.187:42414 I1025 00:03:29.174675 6797 monitor.cpp:140] Failed to collect resource usage for container '6ea903e2-87d0-45a5-8ad1-69924f040402' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: 6ea903e2-87d0-45a5-8ad1-69924f040402 I1025 00:03:29.176897 6787 monitor.cpp:140] Failed to collect resource usage for container 'd19ca0b7-5dd9-4a84-9e92-b72f3b3853ae' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: d19ca0b7-5dd9-4a84-9e92-b72f3b3853ae I1025 00:03:29.485119 6789 slave.cpp:2522] Received ping from slave-observer(2)@67.195.81.187:42414 I1025 00:03:29.618518 6788 hierarchical_allocator_process.hpp:734] Offering cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141024-233658-3142697795-42414-6774-S0 to framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.618857 6788 hierarchical_allocator_process.hpp:816] Filtered mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):2 on slave 20141024-233658-3142697795-42414-6774-S1 for framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.618991 6788 hierarchical_allocator_process.hpp:734] Offering mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1 on slave 20141024-233658-3142697795-42414-6774-S2 to framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.619336 6788 hierarchical_allocator_process.hpp:659] Performed allocation for 3 slaves in 1.120275ms I1025 00:03:29.619598 6787 master.cpp:3795] Sending 2 offers to framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.621063 6788 sched.cpp:544] Scheduler::resourceOffers took 1.235468ms I1025 00:03:29.621443 6788 master.cpp:2321] Processing reply for offers: [ 20141024-233658-3142697795-42414-6774-O952 ] on slave 20141024-233658-3142697795-42414-6774-S2 at slave(3)@67.195.81.187:42414 ( pomona.apache.org) for framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.621809 6788 master.cpp:2321] Processing reply for offers: [ 20141024-233658-3142697795-42414-6774-O953 ] on slave 20141024-233658-3142697795-42414-6774-S0 at slave(2)@67.195.81.187:42414 ( pomona.apache.org) for framework 20141024-233658-3142697795-42414-6774- (Test Framework (Python)) at scheduler-aeb7ad64-b9cf-45b9-8a33-b2a6d292048e@67.195.81.187:42414 I1025 00:03:29.622165 6787 hierarchical_allocator_process.hpp:563] Recovered mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1 (total allocatable: mem(*):10112; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):1) on slave 20141024-233658-3142697795-42414-6774-S2 from framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.622200 6787 hierarchical_allocator_process.hpp:599] Framework 20141024-233658-3142697795-42414-6774- filtered slave 20141024-233658-3142697795-42414-6774-S2 for 5secs I1025 00:03:29.622339 6787 hierarchical_allocator_process.hpp:563] Recovered cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] (total allocatable: cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]) on slave 20141024-233658-3142697795-42414-6774-S0 from framework 20141024-233658-3142697795-42414-6774- I1025 00:03:29.622371 6787 hierarchical_allocator_process.hpp:599] Framework 20141024-233658-3142697795-42414-6774- filtered slave 20141024-233658-3142697795-42414-6774-S0 for 5secs I1025 00:03:29.681506 6797 slave.cpp:2522] Received ping from slave-observer(3)@67.195.81.187:42414 I1025 00:03:30.175997 6795 monitor.cpp:140] Failed to collect resource usage for container '6ea903e2-87d0-45a5-8ad1-69924f040402' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: 6ea903e2-87d0-45a5-8ad1-69924f040402 I1025 00:03:30.178053 6786 monitor.cpp:140] Failed to collect resource usage for container 'd19ca0b7-5dd9-4a84-9e92-b72f3b3853ae' for executor 'default' of framework '20141024-233658-3142697795-42414-6774-': Unknown container: d19ca0b7-5dd9-4a84-9e92-b72f3b3853ae 2014-10-25 00:03:30,187:27064(0x2adb076bf700):ZOO_ERROR@handle_socket_error_msg@1697: Socket
