Re: Review Request 30609: Added a function that reports file size, not following links.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/ --- (Updated March 11, 2015, 10:06 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Changes --- Rebased. Moved os::size() to os::stat::size(). Bugs: MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2072 Repository: mesos Description --- This returns a file's size (on UNIXes as reported by lstat(), not stat()). It is desired that in case of a link, the size of the link, not the size of the referenced file, is returned. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp af940a48b161c194f2efb529b3d589c543b12f61 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp c396c1d2d833b2f1721092fa35b23b5c3c3d99b3 Diff: https://reviews.apache.org/r/30609/diff/ Testing --- Wrote a simple test that creates a file and tests its size, and also checks if a non-existing file yields an error. Thanks, Bernd Mathiske
Build failed in Jenkins: mesos-reviewbot #4529
See https://builds.apache.org/job/mesos-reviewbot/4529/ -- [URLTrigger] A change within the response URL invocation (log) Building remotely on ubuntu-4 (docker Ubuntu ubuntu4 ubuntu) in workspace https://builds.apache.org/job/mesos-reviewbot/ws/ git rev-parse --is-inside-work-tree # timeout=10 Fetching changes from the remote Git repository git config remote.origin.url https://git-wip-us.apache.org/repos/asf/mesos.git # timeout=10 Fetching upstream changes from https://git-wip-us.apache.org/repos/asf/mesos.git git --version # timeout=10 git fetch --tags --progress https://git-wip-us.apache.org/repos/asf/mesos.git +refs/heads/*:refs/remotes/origin/* FATAL: Failed to fetch from https://git-wip-us.apache.org/repos/asf/mesos.git hudson.plugins.git.GitException: Failed to fetch from https://git-wip-us.apache.org/repos/asf/mesos.git at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:647) at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:889) at hudson.plugins.git.GitSCM.checkout(GitSCM.java:914) at hudson.model.AbstractProject.checkout(AbstractProject.java:1252) at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:615) at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86) at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:524) at hudson.model.Run.execute(Run.java:1706) at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43) at hudson.model.ResourceController.execute(ResourceController.java:88) at hudson.model.Executor.run(Executor.java:232) Caused by: hudson.plugins.git.GitException: Command git fetch --tags --progress https://git-wip-us.apache.org/repos/asf/mesos.git +refs/heads/*:refs/remotes/origin/* returned status code 128: stdout: stderr: error: RPC failed; result=6, HTTP code = 0 fatal: The remote end hung up unexpectedly at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1435) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:1223) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$300(CliGitAPIImpl.java:85) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:280) at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:153) at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:146) at hudson.remoting.UserRequest.perform(UserRequest.java:118) at hudson.remoting.UserRequest.perform(UserRequest.java:48) at hudson.remoting.Request$2.run(Request.java:328) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72) at java.util.concurrent.FutureTask.run(FutureTask.java:262) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745)
Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui #2807
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui/2807/changes
Jenkins build is back to normal : mesos-reviewbot #4538
See https://builds.apache.org/job/mesos-reviewbot/4538/
Re: Review Request 31903: Added Python binding for the acceptOffers API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/#review76129 --- Looks good to me :) src/examples/python/test_framework.py https://reviews.apache.org/r/31903/#comment123571 Realized that the Java test framework has a filter here but this one doesn't. Do we want one here too? or is it more or less arbitrary? - Michael Park On March 10, 2015, 6:09 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/ --- (Updated March 10, 2015, 6:09 p.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2428 https://issues.apache.org/jira/browse/MESOS-2428 Repository: mesos Description --- Added Python binding for the acceptOffers API. Diffs - src/examples/python/test_framework.py 27106147900f8b5dd2ea0443f5658902ff1145e4 src/python/interface/src/mesos/interface/__init__.py f3d96a455dc8b66fc4527af1b3dee2f8841b29dd src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp a6980002202b829b40e85188bfb291e5d22c9a26 src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp bb1884597731c73f4815069ceb940cf067790670 Diff: https://reviews.apache.org/r/31903/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review76090 --- src/authentication/cram_md5/authenticator.hpp https://reviews.apache.org/r/27760/#comment123587 i wish this move was done in its own review (w/o functional changes), so that we can commit it right away. src/authentication/cram_md5/authenticator.cpp https://reviews.apache.org/r/27760/#comment123494 looks like AuthenticatorSessionProcess already has an onDiscard handler that transitions the innermost future to FAILED. Do we still need the onDiscard handler here? src/authentication/cram_md5/authenticator.cpp https://reviews.apache.org/r/27760/#comment123495 // The 'error' is set atmost once per os process. src/authentication/cram_md5/auxprop.hpp https://reviews.apache.org/r/27760/#comment123589 lets do this lock protection in its own dependent review. src/master/master.hpp https://reviews.apache.org/r/27760/#comment123497 s/authenticator/authenticator./ src/master/master.hpp https://reviews.apache.org/r/27760/#comment123498 Can you comment on what the outer and inner future signifies? src/master/master.cpp https://reviews.apache.org/r/27760/#comment123499 s/Could not/Failed to/ src/master/master.cpp https://reviews.apache.org/r/27760/#comment123500 s/Cannot/Failed to/ src/master/master.cpp https://reviews.apache.org/r/27760/#comment123501 Send a FrameworkError message (instead of AuthenticationError) here to avoid retries? src/master/master.cpp https://reviews.apache.org/r/27760/#comment123505 s/authenticator/authenticator session/ src/master/master.cpp https://reviews.apache.org/r/27760/#comment123502 authenticated successfully is confusing. do you mean completed authentication process? src/master/master.cpp https://reviews.apache.org/r/27760/#comment123585 Per our offline discussion, I think we can get rid of this promise altogether now. please send a dependent review for that. src/master/master.cpp https://reviews.apache.org/r/27760/#comment123503 Not yours. but can you s/happen/complete/ - Vinod Kone On March 10, 2015, 7:30 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/ --- (Updated March 10, 2015, 7:30 p.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos Description --- The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies. By introducing a two fold design, this has been decoupled from the authentication message processing. The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos. Diffs - include/mesos/authentication/authenticator.hpp f66217a src/Makefile.am 3059818 src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION src/authentication/cram_md5/auxprop.hpp b894386 src/authentication/cram_md5/auxprop.cpp cf503a2 src/master/master.hpp 3c957ab src/master/master.cpp dccd7c6 src/tests/cram_md5_authentication_tests.cpp 92a89c5 Diff: https://reviews.apache.org/r/27760/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31951: Added test for long lived executors.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/
---
(Updated March 11, 2015, 2:17 p.m.)
Review request for mesos, Ben Mahler, Kapil Arya, and switched to 'mcypark'.
Changes
---
Addressed Vinod's comments
Bugs: MESOS-2463
https://issues.apache.org/jira/browse/MESOS-2463
Repository: mesos
Description
---
Environment decorators broke long lived executors during master fail over
(MESOS-2463).
This test verifies the fix suggested in r31889.
Diffs (updated)
-
src/tests/master_tests.cpp acf7050ce69a196ac74627d607b4664060fa1117
Diff: https://reviews.apache.org/r/31951/diff/
Testing
---
make check with and without r31889. (Breaks before with TASK_ERROR similar to
MESOS-2463):
Existing ExecutorInfo:
executor_id {
value: default
}
command {
environment {
}
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Task's ExecutorInfo:
executor_id {
value: default
}
command {
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Thanks,
Niklas Nielsen
Re: Review Request 31951: Added test for long lived executors.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/#review76140
---
Patch looks great!
Reviews applied: [31889, 31951]
All tests passed.
- Mesos ReviewBot
On March 11, 2015, 9:17 p.m., Niklas Nielsen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/
---
(Updated March 11, 2015, 9:17 p.m.)
Review request for mesos, Ben Mahler, Kapil Arya, and switched to 'mcypark'.
Bugs: MESOS-2463
https://issues.apache.org/jira/browse/MESOS-2463
Repository: mesos
Description
---
Environment decorators broke long lived executors during master fail over
(MESOS-2463).
This test verifies the fix suggested in r31889.
Diffs
-
src/tests/master_tests.cpp acf7050ce69a196ac74627d607b4664060fa1117
Diff: https://reviews.apache.org/r/31951/diff/
Testing
---
make check with and without r31889. (Breaks before with TASK_ERROR similar to
MESOS-2463):
Existing ExecutorInfo:
executor_id {
value: default
}
command {
environment {
}
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Task's ExecutorInfo:
executor_id {
value: default
}
command {
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Thanks,
Niklas Nielsen
Jenkins build is back to normal : mesos-reviewbot #4540
See https://builds.apache.org/job/mesos-reviewbot/4540/
Build failed in Jenkins: mesos-reviewbot #4539
See https://builds.apache.org/job/mesos-reviewbot/4539/ -- [...truncated 5606 lines...] sys 5m2.273s + chmod -R +w 3rdparty aclocal.m4 ar-lib autom4te.cache bin bootstrap CHANGELOG compile config.guess config.log config.lt config.status config.sub configure configure.ac depcomp Dockerfile docs Doxyfile ec2 frameworks include install-sh libtool LICENSE ltmain.sh m4 Makefile Makefile.am Makefile.in mesos-0.23.0.tar.gz mesos.pc mesos.pc.in missing mpi NOTICE README.md src support + git clean -fdx Removing .gitignore Removing .libs/ Removing .reviewboardrc Removing 3rdparty/Makefile Removing 3rdparty/Makefile.in Removing 3rdparty/libprocess/.deps/ Removing 3rdparty/libprocess/3rdparty/.deps/ Removing 3rdparty/libprocess/3rdparty/Makefile Removing 3rdparty/libprocess/3rdparty/Makefile.in Removing 3rdparty/libprocess/3rdparty/gmock_sources.cc Removing 3rdparty/libprocess/3rdparty/stout/Makefile Removing 3rdparty/libprocess/3rdparty/stout/Makefile.in Removing 3rdparty/libprocess/3rdparty/stout/aclocal.m4 Removing 3rdparty/libprocess/3rdparty/stout/autom4te.cache/ Removing 3rdparty/libprocess/3rdparty/stout/config.log Removing 3rdparty/libprocess/3rdparty/stout/config.status Removing 3rdparty/libprocess/3rdparty/stout/configure Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile.in Removing 3rdparty/libprocess/3rdparty/stout/missing Removing 3rdparty/libprocess/Makefile Removing 3rdparty/libprocess/Makefile.in Removing 3rdparty/libprocess/aclocal.m4 Removing 3rdparty/libprocess/ar-lib Removing 3rdparty/libprocess/autom4te.cache/ Removing 3rdparty/libprocess/compile Removing 3rdparty/libprocess/config.guess Removing 3rdparty/libprocess/config.log Removing 3rdparty/libprocess/config.lt Removing 3rdparty/libprocess/config.status Removing 3rdparty/libprocess/config.sub Removing 3rdparty/libprocess/configure Removing 3rdparty/libprocess/depcomp Removing 3rdparty/libprocess/include/Makefile Removing 3rdparty/libprocess/include/Makefile.in Removing 3rdparty/libprocess/libtool Removing 3rdparty/libprocess/ltmain.sh Removing 3rdparty/libprocess/m4/libtool.m4 Removing 3rdparty/libprocess/m4/ltoptions.m4 Removing 3rdparty/libprocess/m4/ltsugar.m4 Removing 3rdparty/libprocess/m4/ltversion.m4 Removing 3rdparty/libprocess/m4/lt~obsolete.m4 Removing 3rdparty/libprocess/missing Removing Makefile Removing Makefile.in Removing aclocal.m4 Removing ar-lib Removing autom4te.cache/ Removing bin/gdb-mesos-local.sh Removing bin/gdb-mesos-master.sh Removing bin/gdb-mesos-slave.sh Removing bin/gdb-mesos-tests.sh Removing bin/lldb-mesos-local.sh Removing bin/lldb-mesos-master.sh Removing bin/lldb-mesos-slave.sh Removing bin/lldb-mesos-tests.sh Removing bin/mesos-local-flags.sh Removing bin/mesos-local.sh Removing bin/mesos-master-flags.sh Removing bin/mesos-master.sh Removing bin/mesos-slave-flags.sh Removing bin/mesos-slave.sh Removing bin/mesos-tests-flags.sh Removing bin/mesos-tests.sh Removing bin/mesos.sh Removing bin/valgrind-mesos-local.sh Removing bin/valgrind-mesos-master.sh Removing bin/valgrind-mesos-slave.sh Removing bin/valgrind-mesos-tests.sh Removing compile Removing config.guess Removing config.log Removing config.lt Removing config.status Removing config.sub Removing configure Removing depcomp Removing ec2/Makefile Removing ec2/Makefile.in Removing include/mesos/version.hpp Removing install-sh Removing libtool Removing ltmain.sh Removing m4/libtool.m4 Removing m4/ltoptions.m4 Removing m4/ltsugar.m4 Removing m4/ltversion.m4 Removing m4/lt~obsolete.m4 Removing mesos-0.23.0.tar.gz Removing mesos.pc Removing missing Removing mpi/mpiexec-mesos Removing src/.deps/ Removing src/Makefile Removing src/Makefile.in Removing src/authentication/.deps/ Removing src/authentication/cram_md5/.deps/ Removing src/authorizer/.deps/ Removing src/cli/.deps/ Removing src/common/.deps/ Removing src/containerizer/ Removing src/deploy/mesos-daemon.sh Removing src/deploy/mesos-start-cluster.sh Removing src/deploy/mesos-start-masters.sh Removing src/deploy/mesos-start-slaves.sh Removing src/deploy/mesos-stop-cluster.sh Removing src/deploy/mesos-stop-masters.sh Removing src/deploy/mesos-stop-slaves.sh Removing src/docker/.deps/ Removing src/examples/.deps/ Removing src/examples/java/test-exception-framework Removing src/examples/java/test-executor Removing src/examples/java/test-framework Removing src/examples/java/test-log Removing src/examples/java/test-multiple-executors-framework Removing src/examples/python/test-containerizer Removing src/examples/python/test-executor Removing src/examples/python/test-framework Removing src/exec/.deps/ Removing src/fetcher/ Removing src/files/.deps/ Removing src/health-check/.deps/ Removing src/hook/.deps/ Removing src/java/generated/org/apache/mesos/MesosNativeLibrary.java Removing src/java/jni/.deps/ Removing src/java/mesos.pom Removing src/jvm/.deps/ Removing src/jvm/org/apache/.deps/
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 5:20 p.m., Niklas Nielsen wrote: src/tests/hook_tests.cpp, lines 202-203 https://reviews.apache.org/r/31889/diff/8/?file=891582#file891582line202 Why not launch this as a regular task? This allows us to test just the env decorator hook without any additional bells and whistles. (See Adam's comment above). - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76115 --- On March 11, 2015, 5:49 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 5:49 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 5:49 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- Addressed Nik's comments. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76115 --- include/mesos/hook.hpp https://reviews.apache.org/r/31889/#comment123537 Is this still relevant? src/hook/manager.cpp https://reviews.apache.org/r/31889/#comment123538 s/varialbes/variables/ src/hook/manager.cpp https://reviews.apache.org/r/31889/#comment123569 s/append\/prepend/extend/ src/slave/containerizer/containerizer.cpp https://reviews.apache.org/r/31889/#comment123570 const Environment? src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123535 Why not launch this as a regular task? src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123534 const string - Niklas Nielsen On March 11, 2015, 12:38 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 12:38 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/#review76133
---
Ship it!
Thanks alex, just one minor item and we can get this committed!
src/common/http.cpp
https://reviews.apache.org/r/31700/#comment123581
Can you make these consistent with the ones below?
E.g.
```
{
JSON::Array array;
array.reserve(task.statuses().size()); // MESOS-2353.
foreach (const TaskStatus status, task.statuses()) {
statuses.values.push_back(model(status));
}
object.values[statuses] = array;
}
{
JSON::Array array;
array.reserve(task.labels().labels().size()); // MESOS-2353.
foreach (const Label label, task.labels().labels()) {
labels.values.push_back(JSON::Protobuf(label));
}
object.values[labels] = array;
}
```
src/master/http.cpp
https://reviews.apache.org/r/31700/#comment123582
Do you know if a move is helpful here or if the compiler is already
optimizing this?
```
object.values[slaves] = std::move(array);
```
Don't do it in this change, we need to measure and I'm just curious :)
- Ben Mahler
On March 7, 2015, 1:43 a.m., Alexander Rukletsov wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/
---
(Updated March 7, 2015, 1:43 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-2353
https://issues.apache.org/jira/browse/MESOS-2353
Repository: mesos
Description
---
See summary.
Diffs
-
src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9
src/master/http.cpp b8eef69505b147d4c8a0e005dff545b9fc12a220
Diff: https://reviews.apache.org/r/31700/diff/
Testing
---
make check (OS X 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 5:54 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing (updated) --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 29328: Add option to disable docker containerizer killing orphans
On Jan. 17, 2015, 1:34 a.m., Ben Mahler wrote:
Just curious, what happens to the orphans if you don't kill them? Was there
a ticket for this?
Timothy Chen wrote:
the orphans remains untouched. there is a jira ticket for adding this
flag, i can fimd it later once im next to a computer
Ben Mahler wrote:
I understood that part :)
But why is it ok to leave orphans untouched? Sounds like a bug to me.. is
there some context I'm missing here?
I think the context is that sometimes it's not desirable to remove all orphans
on recovery, especially when the discovery mechanism that a task is launched by
Mesos currently is looking for Docker containers with a mesos- prefix (future
going to be mesos-{slave_id} which is safer).
We want to leave this optionally so if users like to keep the containers and
want to have their own recovery or gc plan we can let them do so.
- Timothy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29328/#review68532
---
On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29328/
---
(Updated Jan. 17, 2015, 1:26 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Repository: mesos
Description
---
Add option to disable docker containerizer killing orphans
Diffs
-
src/slave/containerizer/docker.cpp 5f4b4ce
src/slave/flags.hpp a4498e6
Diff: https://reviews.apache.org/r/29328/diff/
Testing
---
make check
Thanks,
Timothy Chen
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/#review76119 --- 3rdparty/libprocess/src/openssl.cpp https://reviews.apache.org/r/29406/#comment123548 Use the verify flag correctly. Currently it is being ignored and we always verify the cert if provided. - Joris Van Remoortere On Feb. 20, 2015, 7:24 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/ --- (Updated Feb. 20, 2015, 7:24 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1913 https://issues.apache.org/jira/browse/MESOS-1913 Repository: mesos Description --- Requires: configure --enable-libevent --enable-libevent-socket --enable-ssl New environment variables: USE_SSL=(0,1) SSL_CERT=(path to certificate) SSL_KEY=(path to key) SSL_VERIFY_CERT=(0,1) SSL_REQUIRE_CERT=(0,1) SSL_CA_DIR=(path to CA directory) SSL_CA_FILE=(path to CA file) Diffs - 3rdparty/libprocess/Makefile.am 8f96f49a386a70f14324d3a4744aa0b8bf3995f9 3rdparty/libprocess/include/process/socket.hpp ddb9e365fc1e65a568bdac4973964df1ab8cc05e 3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 3rdparty/libprocess/src/libevent.cpp 28c2cf7f49cc153158f2a470a1812e35f7d4b93a 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 3rdparty/libprocess/src/openssl.hpp PRE-CREATION 3rdparty/libprocess/src/openssl.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f Diff: https://reviews.apache.org/r/29406/diff/ Testing --- make check (uses non-ssl socket) benchmarks using ssl sockets master, slave, framework, webui launch with ssl sockets Thanks, Joris Van Remoortere
Re: Review Request 31873: Added Java binding for the new acceptOffers API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/#review76128 --- The Java changes look good to me. I've only skimmed over the C++ JNI changes, JNI code is... painful. Look give a second sweep over the JNI code soon. Meanwhile, just a few nits on comments. src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp https://reviews.apache.org/r/31873/#comment123567 ``` Construct C++ OfferIDs from Java OfferIDs. ``` src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp https://reviews.apache.org/r/31873/#comment123566 ``` // Construct C++ 'Offer::Operation's from Java 'Offer.Operation's. ``` src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp https://reviews.apache.org/r/31873/#comment123565 ``` // Construct C++ Filters from Java Filters. ``` - Michael Park On March 10, 2015, 12:07 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/ --- (Updated March 10, 2015, 12:07 a.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2427 https://issues.apache.org/jira/browse/MESOS-2427 Repository: mesos Description --- Added Java binding for the new acceptOffers API. Diffs - src/examples/java/TestFramework.java 65ba9d9e242f40d6382cd9c18269b87f8b59b5a1 src/java/jni/construct.cpp e54c11ef0ce3e9f6d0e572d3a89cadae417f9cd6 src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 4f0dad78f7300b7bbf147f231eb053fd7faf4b69 src/java/src/org/apache/mesos/MesosSchedulerDriver.java a1055a5d907133485891ebd6d8731c102b913fec src/java/src/org/apache/mesos/SchedulerDriver.java d5b100a4c371bd9c496b9127767c14047185e5f9 Diff: https://reviews.apache.org/r/31873/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 31276: Added cgroup memory pressure listening tests.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31276/#review76118
---
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123543
Looks like you are not using those function directly for now. Could you
please remove them from the header. Keep the comments in the cpp file above the
implementation.
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123564
Do we still need the async signal safe comments?
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123547
The abstraction for controlling the memory usage of a subprocess.
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123561
Return the pid of the subprocess.
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123563
The comments here is not quite clear. Could you please move the comments
above `allocateRSSMemory` to here?
```
// Increase the anonymous memory usage (RSS) of the
// subprocess. The subprocess will use 'mlock' and
// 'memset' to make sure the memory is actually
// mapped by the kernel.
void increaseRSS(const Bytes size);
```
src/tests/memory_test_helper.hpp
https://reviews.apache.org/r/31276/#comment123572
Again, let's rename the function and revise the comments to make it more
easy to understand:
```
// Increases the page cache used by the subprocess. This
// is achieved by writing a temporary file.
TryNothing increasePageCache(const Bytes size);
```
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123574
s/allocateRSSMemory/increaseRSS/
Let's remove the 'lock' parameter and assume all page will be locked. We
can introduce this parameter later once we have a use case.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123579
Please use 'size' in the function arguments and use 'bytes' here:
```
size_t bytes = static_castsize_t(size.bytes());
```
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123575
You don't have to use perror or abort here anymore right? So:
```
TryNothing increaseRSS(...)
{
if (...) {
return Error(...);
}
...
}
```
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123573
Looks like this is not used. Please remove it.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123580
Ditto.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123583
Hum, this makes the page cache increasing depend on the memory allocation.
Can we simply construct a 4K buffer on the stack and perform write multiple
times?
Please do not use any ASSERT or EXPECT in the subprocess. Simply return
Error and let the top level function to deal with error conditions.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123584
You need a comment explaining why fsync is necessary here.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123586
Add a comment about what these constants are.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123588
Please do not use EXPECT or ASSERT.
simply return non-zero and print error messages in stderr
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123550
Instead of putting subprocess spawning logic in the constructor, which
means you have to use ASSERT inside the constructor. Let's defer the assertion
to the actual test. So the interface of MemoryTestHelper will be like:
```
class MemoryTestHelper
{
public:
TryNothing spawn();
Trypid_t pid();
TryNothing increaseRSS(const Bytes size);
TryNothing increasePageCache(const Bytes size);
};
```
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123590
Again, let's not put any gtest related stuff in MemoryTestHelper. Please
return Error instead.
Here and everywhere else.
src/tests/memory_test_helper.cpp
https://reviews.apache.org/r/31276/#comment123596
This is a mix of stop and reap. I would suggest we rename it to 'stop'
which does the kill and wait.
Let's simply kill the process in case the process is stuck.
Again, remove all ASSERT and EXPECT.
src/tests/memory_test_helper_child.hpp
https://reviews.apache.org/r/31276/#comment123544
s/MemoryTestHelperChild/MemoryTestHelperMain/
src/tests/memory_test_helper_child.hpp
https://reviews.apache.org/r/31276/#comment123541
Do you really need this extra file? Can you move this subcommand
Re: Review Request 31873: Added Java binding for the new acceptOffers API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/#review76106 --- Ship it! This patch looks good, but can you please include updates to the CHANGELOG and Upgrades documents in this review chain to capture this API change in 0.23.0? src/examples/java/TestFramework.java https://reviews.apache.org/r/31873/#comment123515 Do you need to fix this in this patch? Just commit the whitespace removal on master if you really want it. src/examples/java/TestFramework.java https://reviews.apache.org/r/31873/#comment123516 Can we move '`launch`' up and use it to store our tasks, instead of using the extra `ListTaskInfo`? src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp https://reviews.apache.org/r/31873/#comment123519 Want to collapse these lines to just `push_back(construct...(...));` src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp https://reviews.apache.org/r/31873/#comment123520 Want to collapse these lines to just `push_back(construct...(...))`? - Ben Mahler On March 10, 2015, 12:07 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/ --- (Updated March 10, 2015, 12:07 a.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2427 https://issues.apache.org/jira/browse/MESOS-2427 Repository: mesos Description --- Added Java binding for the new acceptOffers API. Diffs - src/examples/java/TestFramework.java 65ba9d9e242f40d6382cd9c18269b87f8b59b5a1 src/java/jni/construct.cpp e54c11ef0ce3e9f6d0e572d3a89cadae417f9cd6 src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 4f0dad78f7300b7bbf147f231eb053fd7faf4b69 src/java/src/org/apache/mesos/MesosSchedulerDriver.java a1055a5d907133485891ebd6d8731c102b913fec src/java/src/org/apache/mesos/SchedulerDriver.java d5b100a4c371bd9c496b9127767c14047185e5f9 Diff: https://reviews.apache.org/r/31873/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 31677: stout: Make the counting of netmask set bits more efficient.
On March 11, 2015, 7 p.m., Ben Mahler wrote: I'm curious, what prompted this change? Dominic Hamon wrote: a comment on the original version in a review that it wasn't the best way of counting bits. it seems like a generally useful thing to do. another benefit: IPv6 netmasks, where there are 128 bits. - Evelina --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31677/#review76105 --- On March 10, 2015, 7:59 p.m., Evelina Dumitrescu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31677/ --- (Updated March 10, 2015, 7:59 p.m.) Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van Remoortere, and Niklas Nielsen. Repository: mesos Description --- see summary Diffs - 3rdparty/libprocess/3rdparty/stout/Makefile.am a5224554f6851930aa97cadc5da3d010829d87dc 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ac2bbed6fe86623fb51cac3613d79d7b1372df9d 3rdparty/libprocess/3rdparty/stout/include/stout/bits.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp e4e86deb8a42a1d91f4d4ac210eae26f8f57ee17 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/31677/diff/ Testing --- Thanks, Evelina Dumitrescu
Re: Review Request 30774: Fetcher Cache
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 11, 2015, 10:50 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Changes --- Rebased. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only one download. The fetcher program remains external for safety reasons, but is now augmented with more elaborate parameters packed into a JSON object to implement specific fetch actions for all of the above. Additional testing includes fetching from (mock) HDFS and coverage of the new features. Diffs (updated) - docs/configuration.md fc3afec248b534b1d5eb625eb66de5f90cd8cd33 docs/fetcher-cache-internals.md PRE-CREATION docs/fetcher.md PRE-CREATION include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/Makefile.am 3059818231c46484039d179cd6916932eff6cd68 src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp fbd1c0a0e5f4f227adb022f0baaa6d2c7e3ad748 src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/docker_containerizer_tests.cpp 06cd3d89ecbaaac17ae6970604b21fbe29f6e887 src/tests/fetcher_cache_tests.cpp PRE-CREATION src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e src/tests/mesos.hpp 45e35204d1aa876fa0c871acf0f21afcd5ababe8 src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1 Diff: https://reviews.apache.org/r/30774/diff/ Testing --- make check --- longer Description: --- -Replaces all other reviews for the fetcher cache except those related to stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 30618, 30621, 30626. See descriptions of those. In dependency order: 30033: Removes the fetcher env tests since these won't be needed any more when the fetcher uses JSON in a single env var as a parameter. They never tested anything that won't be covered by other tests anyway. 30034: Makes the code structure of all fetcher tests the same. Instead of calling the run method of the fetcher directly, calling through fetch(). Also removes all uses of I/O redirection, which is not really needed for debugging, and thus the next patch can refactor fetch() and run(). (The latter comes in two varieties, which complicates matters without much benefit.) 30036: Extends the CommandInfo::URI protobuf with a boolean caching field that will later cause fetcher cache actions. Also introduces the notion of a cache directory to the fetcher info protobuf. And then propagates these additions throughout the rest of the code base where applicable. This includes passing the slave ID all the way down to the place where the cache dir name is constructed. 30037: Extends the fetcher info protobuf with actions (fetch directly bypassing the cache, fetch through the cache, retrieve from the cache). Switches the basis for dealing with uris to items, which contain the uri, the action, and potentially a cache file name. Refactors fetch() and run(), so there is only one of each. Introduces about half of the actual cache logic, including a hashmap of cache file objects for bookkeeping and basic operations on it. 30039: Enables fetcher cache actions in the mesos fetcher program. 30006: Enables concurrent
Jenkins build is back to normal : mesos-reviewbot #4533
See https://builds.apache.org/job/mesos-reviewbot/4533/
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 1:02 a.m., Adam B wrote: src/tests/hook_tests.cpp, line 199 https://reviews.apache.org/r/31889/diff/6/?file=891026#file891026line199 Does this being a HookTest mean that it's guaranteed to use the FOO=bar test executorEnvironmentDecorator hook? Kapil Arya wrote: I am not sure if I understand your question here. The test hook sets FOO=bar in the slaveExecutorEnvironmentDecorator and that's what we verify here. This test (along with other tests) is tightly coupled with the test hook module. Ok. Just wanted to make sure that this test (and other HookTests, I guess) guarantees that the test hook is loaded, without any special build/command-line parameters to make check. - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76049 --- On March 11, 2015, 12:08 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 12:08 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 31903: Added Python binding for the acceptOffers API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/#review76110 --- Ship it! This patch looks good, but can you please include updates to the CHANGELOG and Upgrades documents in this review chain to capture this API change in 0.23.0? src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp https://reviews.apache.org/r/31903/#comment123529 Can we get some newlines here? src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp https://reviews.apache.org/r/31903/#comment123531 newline here? src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp https://reviews.apache.org/r/31903/#comment123530 Can we get some newlines here? src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp https://reviews.apache.org/r/31903/#comment123532 newline here? - Ben Mahler On March 10, 2015, 6:09 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/ --- (Updated March 10, 2015, 6:09 p.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2428 https://issues.apache.org/jira/browse/MESOS-2428 Repository: mesos Description --- Added Python binding for the acceptOffers API. Diffs - src/examples/python/test_framework.py 27106147900f8b5dd2ea0443f5658902ff1145e4 src/python/interface/src/mesos/interface/__init__.py f3d96a455dc8b66fc4527af1b3dee2f8841b29dd src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp a6980002202b829b40e85188bfb291e5d22c9a26 src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp bb1884597731c73f4815069ceb940cf067790670 Diff: https://reviews.apache.org/r/31903/diff/ Testing --- make check Thanks, Jie Yu
Build failed in Jenkins: mesos-reviewbot #4532
See https://builds.apache.org/job/mesos-reviewbot/4532/ -- [URLTrigger] A change within the response URL invocation (log) Building remotely on ubuntu-1 (docker Ubuntu ubuntu ubuntu1) in workspace https://builds.apache.org/job/mesos-reviewbot/ws/ git rev-parse --is-inside-work-tree # timeout=10 Fetching changes from the remote Git repository git config remote.origin.url https://git-wip-us.apache.org/repos/asf/mesos.git # timeout=10 Fetching upstream changes from https://git-wip-us.apache.org/repos/asf/mesos.git git --version # timeout=10 git fetch --tags --progress https://git-wip-us.apache.org/repos/asf/mesos.git +refs/heads/*:refs/remotes/origin/* FATAL: Failed to fetch from https://git-wip-us.apache.org/repos/asf/mesos.git hudson.plugins.git.GitException: Failed to fetch from https://git-wip-us.apache.org/repos/asf/mesos.git at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:647) at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:889) at hudson.plugins.git.GitSCM.checkout(GitSCM.java:914) at hudson.model.AbstractProject.checkout(AbstractProject.java:1252) at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:615) at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86) at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:524) at hudson.model.Run.execute(Run.java:1706) at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43) at hudson.model.ResourceController.execute(ResourceController.java:88) at hudson.model.Executor.run(Executor.java:232) Caused by: hudson.plugins.git.GitException: Command git fetch --tags --progress https://git-wip-us.apache.org/repos/asf/mesos.git +refs/heads/*:refs/remotes/origin/* returned status code 128: stdout: stderr: fatal: unable to access 'https://git-wip-us.apache.org/repos/asf/mesos.git/': Could not resolve host: git-wip-us.apache.org at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1435) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:1223) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$300(CliGitAPIImpl.java:85) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:280) at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:153) at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:146) at hudson.remoting.UserRequest.perform(UserRequest.java:118) at hudson.remoting.UserRequest.perform(UserRequest.java:48) at hudson.remoting.Request$2.run(Request.java:328) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72) at java.util.concurrent.FutureTask.run(FutureTask.java:262) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745)
Re: Review Request 31951: Added test for long lived executors.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/#review76104
---
Bad patch!
Reviews applied: [31889]
Failed command: ./support/apply-review.sh -n -r 31889
Error:
2015-03-11 18:51:59 URL:https://reviews.apache.org/r/31889/diff/raw/
[12515/12515] - 31889.patch [1]
error: patch failed: src/examples/test_hook_module.cpp:101
error: src/examples/test_hook_module.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On March 11, 2015, 6:43 p.m., Niklas Nielsen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/
---
(Updated March 11, 2015, 6:43 p.m.)
Review request for mesos, Ben Mahler, Kapil Arya, and switched to 'mcypark'.
Bugs: MESOS-2463
https://issues.apache.org/jira/browse/MESOS-2463
Repository: mesos
Description
---
Environment decorators broke long lived executors during master fail over
(MESOS-2463).
This test verifies the fix suggested in r31889.
Diffs
-
src/tests/master_tests.cpp acf7050ce69a196ac74627d607b4664060fa1117
Diff: https://reviews.apache.org/r/31951/diff/
Testing
---
make check with and without r31889. (Breaks before with TASK_ERROR similar to
MESOS-2463):
Existing ExecutorInfo:
executor_id {
value: default
}
command {
environment {
}
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Task's ExecutorInfo:
executor_id {
value: default
}
command {
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Thanks,
Niklas Nielsen
Jenkins build is back to normal : mesos-reviewbot #4535
See https://builds.apache.org/job/mesos-reviewbot/4535/changes
Re: Review Request 31677: stout: Make the counting of netmask set bits more efficient.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31677/#review76105 --- I'm curious, what prompted this change? 3rdparty/libprocess/3rdparty/stout/include/stout/bits.hpp https://reviews.apache.org/r/31677/#comment123510 Can we return a size_t to capture that this is a count and that negatives are not possible? Is `bits::count` not a sufficient name? - Ben Mahler On March 10, 2015, 7:59 p.m., Evelina Dumitrescu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31677/ --- (Updated March 10, 2015, 7:59 p.m.) Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van Remoortere, and Niklas Nielsen. Repository: mesos Description --- see summary Diffs - 3rdparty/libprocess/3rdparty/stout/Makefile.am a5224554f6851930aa97cadc5da3d010829d87dc 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ac2bbed6fe86623fb51cac3613d79d7b1372df9d 3rdparty/libprocess/3rdparty/stout/include/stout/bits.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp e4e86deb8a42a1d91f4d4ac210eae26f8f57ee17 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/31677/diff/ Testing --- Thanks, Evelina Dumitrescu
Re: Review Request 31905: Fixed protobuf comparisons by accounting for new fields.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31905/#review76084
---
src/common/type_utils.cpp
https://reviews.apache.org/r/31905/#comment123489
So we are ignoring CommandInfo::ContainerInfo because it's being
deprecated? If yes, could you please add a NOTE here?
src/common/type_utils.cpp
https://reviews.apache.org/r/31905/#comment123488
Kill the extra line here.
src/common/type_utils.cpp
https://reviews.apache.org/r/31905/#comment123493
Looks like we have the same logic for many repeated fields here. I am
wondering if we can have a helper function:
```
template typename T
bool equivalent(
const RepeatedFieldT left,
const RepeatedFieldT right,
bool checkOrder = false)
{
if (left.size() != right.size()) {
return false;
}
for (int i = 0; i left.size(); i++) {
...
}
...
}
```
- Jie Yu
On March 11, 2015, 12:37 a.m., Vinod Kone wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31905/
---
(Updated March 11, 2015, 12:37 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, Joerg Schad, and Timothy Chen.
Bugs: MESOS-2309
https://issues.apache.org/jira/browse/MESOS-2309
Repository: mesos
Description
---
When new fields were added to protobufs these operators were not updated.
Fixed now.
Diffs
-
src/common/type_utils.cpp a1704c67d04d19f65d94dbe56a61bb28561e5bf3
Diff: https://reviews.apache.org/r/31905/diff/
Testing
---
make check
Thanks,
Vinod Kone
Build failed in Jenkins: mesos-reviewbot #4534
See https://builds.apache.org/job/mesos-reviewbot/4534/
--
[...truncated 5576 lines...]
rm -rf state/.libs state/_libs
rm -rf usage/.libs usage/_libs
rm -rf watcher/.libs watcher/_libs
rm -rf zookeeper/.libs zookeeper/_libs
rm -rf ./.deps authentication/.deps authentication/cram_md5/.deps
authorizer/.deps cli/.deps common/.deps containerizer/.deps docker/.deps
examples/.deps exec/.deps fetcher/.deps files/.deps health-check/.deps
hook/.deps java/jni/.deps jvm/.deps jvm/org/apache/.deps launcher/.deps
linux/.deps linux/routing/.deps linux/routing/diagnosis/.deps
linux/routing/filter/.deps linux/routing/link/.deps
linux/routing/queueing/.deps local/.deps log/.deps log/tool/.deps logging/.deps
master/.deps master/allocator/sorter/drf/.deps messages/.deps module/.deps
sched/.deps scheduler/.deps slave/.deps slave/containerizer/.deps
slave/containerizer/isolators/cgroups/.deps
slave/containerizer/isolators/filesystem/.deps
slave/containerizer/isolators/namespaces/.deps
slave/containerizer/isolators/network/.deps
slave/containerizer/isolators/posix/.deps slave/containerizer/mesos/.deps
state/.deps tests/.deps tests/common/.deps usage/.deps watcher/.deps
zookeeper/.deps
rm -f Makefile
make[2]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/src'
Making distclean in ec2
make[2]: Entering directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/ec2'
rm -rf .libs _libs
rm -f *.lo
test -z || rm -f
test . = ../../ec2 || test -z || rm -f
rm -f Makefile
make[2]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/ec2'
rm -f config.status config.cache config.log configure.lineno
config.status.lineno
rm -f Makefile
make[1]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build'
if test -d mesos-0.23.0; then find mesos-0.23.0 -type d ! -perm -200 -exec
chmod u+w {} ';' rm -rf mesos-0.23.0 || { sleep 5 rm -rf
mesos-0.23.0; }; else :; fi
==
mesos-0.23.0 archives ready for distribution:
mesos-0.23.0.tar.gz
==
real11m40.722s
user65m36.175s
sys 4m53.496s
+ chmod -R +w 3rdparty aclocal.m4 ar-lib autom4te.cache bin bootstrap CHANGELOG
compile config.guess config.log config.lt config.status config.sub configure
configure.ac depcomp Dockerfile docs Doxyfile ec2 frameworks include install-sh
libtool LICENSE ltmain.sh m4 Makefile Makefile.am Makefile.in
mesos-0.23.0.tar.gz mesos.pc mesos.pc.in missing mpi NOTICE README.md src
support
+ git clean -fdx
Removing .gitignore
Removing .libs/
Removing .reviewboardrc
Removing 3rdparty/Makefile
Removing 3rdparty/Makefile.in
Removing 3rdparty/libprocess/.deps/
Removing 3rdparty/libprocess/3rdparty/.deps/
Removing 3rdparty/libprocess/3rdparty/Makefile
Removing 3rdparty/libprocess/3rdparty/Makefile.in
Removing 3rdparty/libprocess/3rdparty/gmock_sources.cc
Removing 3rdparty/libprocess/3rdparty/stout/Makefile
Removing 3rdparty/libprocess/3rdparty/stout/Makefile.in
Removing 3rdparty/libprocess/3rdparty/stout/aclocal.m4
Removing 3rdparty/libprocess/3rdparty/stout/autom4te.cache/
Removing 3rdparty/libprocess/3rdparty/stout/config.log
Removing 3rdparty/libprocess/3rdparty/stout/config.status
Removing 3rdparty/libprocess/3rdparty/stout/configure
Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile
Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile.in
Removing 3rdparty/libprocess/3rdparty/stout/missing
Removing 3rdparty/libprocess/Makefile
Removing 3rdparty/libprocess/Makefile.in
Removing 3rdparty/libprocess/aclocal.m4
Removing 3rdparty/libprocess/ar-lib
Removing 3rdparty/libprocess/autom4te.cache/
Removing 3rdparty/libprocess/compile
Removing 3rdparty/libprocess/config.guess
Removing 3rdparty/libprocess/config.log
Removing 3rdparty/libprocess/config.lt
Removing 3rdparty/libprocess/config.status
Removing 3rdparty/libprocess/config.sub
Removing 3rdparty/libprocess/configure
Removing 3rdparty/libprocess/depcomp
Removing 3rdparty/libprocess/include/Makefile
Removing 3rdparty/libprocess/include/Makefile.in
Removing 3rdparty/libprocess/libtool
Removing 3rdparty/libprocess/ltmain.sh
Removing 3rdparty/libprocess/m4/libtool.m4
Removing 3rdparty/libprocess/m4/ltoptions.m4
Removing 3rdparty/libprocess/m4/ltsugar.m4
Removing 3rdparty/libprocess/m4/ltversion.m4
Removing 3rdparty/libprocess/m4/lt~obsolete.m4
Removing 3rdparty/libprocess/missing
Removing Makefile
Removing Makefile.in
Removing aclocal.m4
Removing ar-lib
Removing autom4te.cache/
Removing bin/gdb-mesos-local.sh
Removing bin/gdb-mesos-master.sh
Removing bin/gdb-mesos-slave.sh
Removing bin/gdb-mesos-tests.sh
Removing bin/lldb-mesos-local.sh
Removing bin/lldb-mesos-master.sh
Removing bin/lldb-mesos-slave.sh
Removing bin/lldb-mesos-tests.sh
Removing bin/mesos-local-flags.sh
Removing bin/mesos-local.sh
Removing bin/mesos-master-flags.sh
Review Request 31951: Added test for long lived executors.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31951/
---
Review request for mesos, Ben Mahler, Kapil Arya, and switched to 'mcypark'.
Bugs: MESOS-2463
https://issues.apache.org/jira/browse/MESOS-2463
Repository: mesos
Description
---
Environment decorators broke long lived executors during master fail over
(MESOS-2463).
This test verifies the fix suggested in r31889.
Diffs
-
src/tests/master_tests.cpp acf7050ce69a196ac74627d607b4664060fa1117
Diff: https://reviews.apache.org/r/31951/diff/
Testing
---
make check with and without r31889. (Breaks before with TASK_ERROR similar to
MESOS-2463):
Existing ExecutorInfo:
executor_id {
value: default
}
command {
environment {
}
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Task's ExecutorInfo:
executor_id {
value: default
}
command {
value: exit 1
}
framework_id {
value: 20150311-114156-3758096394-50894-29122-
}
Thanks,
Niklas Nielsen
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 3:08 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- Addressed Nik's and Adam's comments. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 31904: Fixed comparison of protobufs with optional fields.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31904/#review76082 --- Ship it! Ship It! - Jie Yu On March 10, 2015, 6:21 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31904/ --- (Updated March 10, 2015, 6:21 p.m.) Review request for mesos, Ben Mahler, Jie Yu, and Joerg Schad. Bugs: MESOS-2309 https://issues.apache.org/jira/browse/MESOS-2309 Repository: mesos Description --- Now optional fields are compared in the same way as required fields. I changed the order of fields to match the tag numbers for consistency. Diffs - src/common/type_utils.cpp a1704c67d04d19f65d94dbe56a61bb28561e5bf3 Diff: https://reviews.apache.org/r/31904/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 1:30 p.m., Niklas Nielsen wrote: include/mesos/hook.hpp, lines 55-56 https://reviews.apache.org/r/31889/diff/6/?file=891020#file891020line55 This is a public API; let's defer this todo to a JIRA if we plan to do something about it This is not a TODO, this is a guideline for the module writer. On March 11, 2015, 1:30 p.m., Niklas Nielsen wrote: src/hook/manager.hpp, line 50 https://reviews.apache.org/r/31889/diff/6/?file=891022#file891022line50 Do we need to pass the 'set' environment? I think the API would be cleaner if we only talked about environment variables in terms of the Environment proto. Yes, otherwise if two modules change the same environment variable (e.g., PATH), only the last change will survive. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76079 --- On March 11, 2015, 3:08 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 3:08 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Build failed in Jenkins: mesos-reviewbot #4531
See https://builds.apache.org/job/mesos-reviewbot/4531/
--
[...truncated 5547 lines...]
rm -rf linux/routing/diagnosis/.libs linux/routing/diagnosis/_libs
rm -rf linux/routing/filter/.libs linux/routing/filter/_libs
rm -rf linux/routing/link/.libs linux/routing/link/_libs
rm -rf linux/routing/queueing/.libs linux/routing/queueing/_libs
rm -rf local/.libs local/_libs
rm -rf log/.libs log/_libs
rm -rf log/tool/.libs log/tool/_libs
rm -rf logging/.libs logging/_libs
rm -rf master/.libs master/_libs
rm -rf master/allocator/sorter/drf/.libs master/allocator/sorter/drf/_libs
rm -rf messages/.libs messages/_libs
rm -rf module/.libs module/_libs
rm -rf sched/.libs sched/_libs
rm -rf scheduler/.libs scheduler/_libs
rm -rf slave/.libs slave/_libs
rm -rf slave/containerizer/.libs slave/containerizer/_libs
rm -rf slave/containerizer/isolators/cgroups/.libs
slave/containerizer/isolators/cgroups/_libs
rm -f tests/common/*.o
rm -rf slave/containerizer/isolators/filesystem/.libs
slave/containerizer/isolators/filesystem/_libs
rm -f usage/*.o
rm -rf slave/containerizer/isolators/namespaces/.libs
slave/containerizer/isolators/namespaces/_libs
rm -f usage/*.lo
rm -rf slave/containerizer/isolators/network/.libs
slave/containerizer/isolators/network/_libs
rm -f watcher/*.o
rm -rf slave/containerizer/isolators/posix/.libs
slave/containerizer/isolators/posix/_libs
rm -f watcher/*.lo
rm -rf slave/containerizer/mesos/.libs slave/containerizer/mesos/_libs
rm -f zookeeper/*.o
rm -f zookeeper/*.lo
rm -rf state/.libs state/_libs
rm -rf usage/.libs usage/_libs
rm -rf watcher/.libs watcher/_libs
rm -rf zookeeper/.libs zookeeper/_libs
rm -rf ./.deps authentication/.deps authentication/cram_md5/.deps
authorizer/.deps cli/.deps common/.deps containerizer/.deps docker/.deps
examples/.deps exec/.deps fetcher/.deps files/.deps health-check/.deps
hook/.deps java/jni/.deps jvm/.deps jvm/org/apache/.deps launcher/.deps
linux/.deps linux/routing/.deps linux/routing/diagnosis/.deps
linux/routing/filter/.deps linux/routing/link/.deps
linux/routing/queueing/.deps local/.deps log/.deps log/tool/.deps logging/.deps
master/.deps master/allocator/sorter/drf/.deps messages/.deps module/.deps
sched/.deps scheduler/.deps slave/.deps slave/containerizer/.deps
slave/containerizer/isolators/cgroups/.deps
slave/containerizer/isolators/filesystem/.deps
slave/containerizer/isolators/namespaces/.deps
slave/containerizer/isolators/network/.deps
slave/containerizer/isolators/posix/.deps slave/containerizer/mesos/.deps
state/.deps tests/.deps tests/common/.deps usage/.deps watcher/.deps
zookeeper/.deps
rm -f Makefile
make[2]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/src'
Making distclean in ec2
make[2]: Entering directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/ec2'
rm -rf .libs _libs
rm -f *.lo
test -z || rm -f
test . = ../../ec2 || test -z || rm -f
rm -f Makefile
make[2]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build/ec2'
rm -f config.status config.cache config.log configure.lineno
config.status.lineno
rm -f Makefile
make[1]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.23.0/_build'
if test -d mesos-0.23.0; then find mesos-0.23.0 -type d ! -perm -200 -exec
chmod u+w {} ';' rm -rf mesos-0.23.0 || { sleep 5 rm -rf
mesos-0.23.0; }; else :; fi
==
mesos-0.23.0 archives ready for distribution:
mesos-0.23.0.tar.gz
==
real11m26.368s
user64m25.392s
sys 4m45.343s
+ chmod -R +w 3rdparty aclocal.m4 ar-lib autom4te.cache bin bootstrap CHANGELOG
compile config.guess config.log config.lt config.status config.sub configure
configure.ac depcomp Dockerfile docs Doxyfile ec2 frameworks include install-sh
libtool LICENSE ltmain.sh m4 Makefile Makefile.am Makefile.in
mesos-0.23.0.tar.gz mesos.pc mesos.pc.in missing mpi NOTICE README.md src
support
+ git clean -fdx
Removing .gitignore
Removing .libs/
Removing .reviewboardrc
Removing 3rdparty/Makefile
Removing 3rdparty/Makefile.in
Removing 3rdparty/libprocess/.deps/
Removing 3rdparty/libprocess/3rdparty/.deps/
Removing 3rdparty/libprocess/3rdparty/Makefile
Removing 3rdparty/libprocess/3rdparty/Makefile.in
Removing 3rdparty/libprocess/3rdparty/gmock_sources.cc
Removing 3rdparty/libprocess/3rdparty/stout/Makefile
Removing 3rdparty/libprocess/3rdparty/stout/Makefile.in
Removing 3rdparty/libprocess/3rdparty/stout/aclocal.m4
Removing 3rdparty/libprocess/3rdparty/stout/autom4te.cache/
Removing 3rdparty/libprocess/3rdparty/stout/config.log
Removing 3rdparty/libprocess/3rdparty/stout/config.status
Removing 3rdparty/libprocess/3rdparty/stout/configure
Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile
Removing 3rdparty/libprocess/3rdparty/stout/include/Makefile.in
Removing
Requesting ability to contribute for issue MESOS-2479
Greetings! I just filed https://issues.apache.org/jira/browse/MESOS-2479 because I'd like to fix a minor, but very obnoxious and highly visible bug where the search filter disappears if you enter a string that yields no results. I have the fix ready to push for review. Thanks, Joe
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
On March 11, 2015, 9:38 p.m., Ben Mahler wrote: src/master/http.cpp, line 471 https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471 Do you know if a move is helpful here or if the compiler is already optimizing this? ``` object.values[slaves] = std::move(array); ``` Don't do it in this change, we need to measure and I'm just curious :) Alexander Rukletsov wrote: I believe the compiler won't optimize this, since `array` is an lvalue and may be used after this assignment. One thing how we can try to persuade the compiler to move without writing `std::move()` is to wrap array population in a function that returns an array, this will be a rvalue then and c++11-capable compiler *should* choose the right overload, that takes a rvalue ref: http://www.cplusplus.com/reference/map/map/insert/. But everything more complex than `.reserve()` should be benchmarked, on all official supported compilers if possible. I'll play with this when I have some free cycles. Cody Maloney wrote: From writing my own JSON code, the std::move here is definitely necessary to do this as quickly and cheaply as possible. Using std::move here I think is actually the right thing to do, rather than trying to convince the compiler via other means, it's explicitly saying to the reader This variable you were using earlier, it is having it's contents ripped out of it. Putting it into a function and guaranteeing NRVO or RVO fire so that you get the cheap move insertion has a lot of things people can disturb only slightly and end up breaking it. Updating the instance in stout/protobuf.hpp where the JSON::Array is copied to be a move would also probably help significantly. The way JSON::Protobuf's api is setup with Operator() forces a copy of the JSON::Object after it is fully constructed which is rather expensive. Would be good to make JSON::Protobuf() just be a function which can then use the object internally, and move out the result rather than forcing the full object copy. - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/#review76133 --- On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/ --- (Updated March 11, 2015, 10:40 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2353 https://issues.apache.org/jira/browse/MESOS-2353 Repository: mesos Description --- See summary. Diffs - src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9 src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24 Diff: https://reviews.apache.org/r/31700/diff/ Testing --- make check (OS X 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Review Request 31960: Added concurrency protection within the SASL auxprop plugin code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/ --- Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/authentication/cram_md5/auxprop.hpp b894386 src/authentication/cram_md5/auxprop.cpp cf503a2 Diff: https://reviews.apache.org/r/31960/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 7:34 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- Updated TODOs to clarify conflicts. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 7:06 p.m., Adam B wrote: src/slave/containerizer/containerizer.cpp, lines 294-297 https://reviews.apache.org/r/31889/diff/9-11/?file=891672#file891672line294 Not sure this TODO is accurate/necessary anymore. This patch assumes that the decorator hook returns the entire env from executorInfo as well as its own modifications, so it's up to the hook to decide how to resolve conflicts. On the other hand, if the hook incorrectly returns only its own environment, then the executorInfo.environment variables will be silently dropped. Kapil Arya wrote: The hook still returns only it's own environment variables. The hook manager merges the returned values with the executorInfo's environment and finally returns an aggregate. However, the caller can still overwrite anything in the aggregated result by overwriting with the values from the original executorInfo's environment. Updated the TODO comment to clarify the situation. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76158 --- On March 11, 2015, 7:34 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 7:34 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 30546: MemIsolator: expose memory pressures for containers.
On March 11, 2015, 12:02 a.m., Jie Yu wrote: src/tests/slave_recovery_tests.cpp, lines 3568-3569 https://reviews.apache.org/r/30546/diff/2/?file=881465#file881465line3568 I suggest moving this test to cgroups_isolator_tests.cpp, or even create a cgroups_memory_isolator_tests.cpp and move tests accordingly. I don't think we should move all slave recovery related tests to a single file, especially the test is for testing the slave recovery for a special isolator. as discussed: left a TODO. will do a separate patch for that. On March 11, 2015, 12:02 a.m., Jie Yu wrote: src/tests/slave_recovery_tests.cpp, lines 3667-3670 https://reviews.apache.org/r/30546/diff/2/?file=881465#file881465line3667 Can you test the slave recovery in a separate test? dropped as discussed. On March 11, 2015, 12:02 a.m., Jie Yu wrote: src/tests/slave_recovery_tests.cpp, lines 3661-3662 https://reviews.apache.org/r/30546/diff/2/?file=881465#file881465line3661 Just do a ``` ASSERT_LE(waited, Seconds(5)); ``` i dropped the logging but kept the EXPECT to allow further clean up if it failed. It is also consistent with other tests where this patten is used. - Chi --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/#review75951 --- On Feb. 28, 2015, 1:43 a.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/ --- (Updated Feb. 28, 2015, 1:43 a.m.) Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-2136 https://issues.apache.org/jira/browse/MESOS-2136 Repository: mesos Description --- MemIsolator: expose memory pressures for containers. Diffs - include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/slave/containerizer/isolators/cgroups/mem.hpp a00f723de61b9bccd76a2948b6d14fde7a993a8d src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e src/tests/slave_recovery_tests.cpp 53adae0118a26e6d25a9ff20c6374cc8e73275b1 Diff: https://reviews.apache.org/r/30546/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 6:51 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- Added another TODO to address Adam's comment. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 6:30 p.m., Adam B wrote: src/slave/containerizer/containerizer.cpp, lines 295-302 https://reviews.apache.org/r/31889/diff/9/?file=891672#file891672line295 What about other env[] variables set earlier in this function? If the decorator wants to check/modify MESOS_DIRECTORY or other MESOS_foo env variables, it won't know what they are being set to and will have to blindly override them if it wants to set them. Maybe we could pass a mutable copy of ExecutorInfo where the env[] variables have been merged in, or go back to passing env[] itself, after the executorInfo defaults have been set. Niklas Nielsen wrote: A nice to have for now IMO. We can leave a comment/TODO/JIRA but I think we should discuss whether we actually want the decorator to change internal environment variables. Added a TODO. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76147 --- On March 11, 2015, 6:51 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 6:51 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 30546: MemIsolator: expose memory pressures for containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/#review76159 --- Patch looks great! Reviews applied: [31008, 30545, 30546] All tests passed. - Mesos ReviewBot On March 11, 2015, 10:32 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/ --- (Updated March 11, 2015, 10:32 p.m.) Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-2136 https://issues.apache.org/jira/browse/MESOS-2136 Repository: mesos Description --- MemIsolator: expose memory pressures for containers. Diffs - include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/slave/containerizer/isolators/cgroups/mem.hpp a00f723de61b9bccd76a2948b6d14fde7a993a8d src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e src/tests/slave_recovery_tests.cpp 53adae0118a26e6d25a9ff20c6374cc8e73275b1 Diff: https://reviews.apache.org/r/30546/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/ --- (Updated March 11, 2015, 11:17 p.m.) Review request for Benjamin Hindman and Niklas Nielsen. Changes --- 1) Fix verify flag 2) Use create pattern like in poll socket 3) Rename postInit to initialize 4) Rename eventDtor to finalize 5) Expand comment for 'eventLoopHandle' weak_ptr 7) Add comment to and rename freeSSLCtx to 'accepted' 8) Add comment to _accept() continuation 9) Bind acceptRequest members earlier, so they don't have to be passed through acceptcallback() Bugs: MESOS-1913 https://issues.apache.org/jira/browse/MESOS-1913 Repository: mesos Description --- Requires: configure --enable-libevent --enable-libevent-socket --enable-ssl New environment variables: USE_SSL=(0,1) SSL_CERT=(path to certificate) SSL_KEY=(path to key) SSL_VERIFY_CERT=(0,1) SSL_REQUIRE_CERT=(0,1) SSL_CA_DIR=(path to CA directory) SSL_CA_FILE=(path to CA file) Diffs (updated) - 3rdparty/libprocess/Makefile.am 8f96f49a386a70f14324d3a4744aa0b8bf3995f9 3rdparty/libprocess/include/process/socket.hpp ddb9e365fc1e65a568bdac4973964df1ab8cc05e 3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 3rdparty/libprocess/src/libevent.cpp 28c2cf7f49cc153158f2a470a1812e35f7d4b93a 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 3rdparty/libprocess/src/openssl.hpp PRE-CREATION 3rdparty/libprocess/src/openssl.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f Diff: https://reviews.apache.org/r/29406/diff/ Testing --- make check (uses non-ssl socket) benchmarks using ssl sockets master, slave, framework, webui launch with ssl sockets Thanks, Joris Van Remoortere
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/#review76171 --- Patch looks great! Reviews applied: [31700] All tests passed. - Mesos ReviewBot On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/ --- (Updated March 11, 2015, 10:40 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2353 https://issues.apache.org/jira/browse/MESOS-2353 Repository: mesos Description --- See summary. Diffs - src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9 src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24 Diff: https://reviews.apache.org/r/31700/diff/ Testing --- make check (OS X 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76172 --- Ship it! Ship It! - Adam B On March 11, 2015, 4:42 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 4:42 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/ --- (Updated March 12, 2015, 12:32 a.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. Changes --- Split review into dependency chain and addressed a couple of comments. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos Description --- The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies. By introducing a two fold design, this has been decoupled from the authentication message processing. The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos. Diffs (updated) - include/mesos/authentication/authenticator.hpp f66217a src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION src/authentication/cram_md5/auxprop.hpp b894386 src/master/master.hpp 3c957ab src/master/master.cpp dccd7c6 src/tests/cram_md5_authentication_tests.cpp 92a89c5 Diff: https://reviews.apache.org/r/27760/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/authentication/cram_md5/authenticator.hpp, lines 79-82 https://reviews.apache.org/r/27760/diff/23/?file=890570#file890570line79 i wish this move was done in its own review (w/o functional changes), so that we can commit it right away. Fixed that, now we got https://reviews.apache.org/r/31961/ On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/authentication/cram_md5/auxprop.hpp, line 54 https://reviews.apache.org/r/27760/diff/23/?file=890572#file890572line54 lets do this lock protection in its own dependent review. Fixed that, now we got https://reviews.apache.org/r/31960/ On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/master/master.hpp, lines 668-670 https://reviews.apache.org/r/27760/diff/23/?file=890574#file890574line668 Can you comment on what the outer and inner future signifies? We got rid of them :) On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/master/master.cpp, lines 3888-3889 https://reviews.apache.org/r/27760/diff/23/?file=890575#file890575line3888 Per our offline discussion, I think we can get rid of this promise altogether now. please send a dependent review for that. Fixed that, we now got https://reviews.apache.org/r/31957/ On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/master/master.cpp, line 3887 https://reviews.apache.org/r/27760/diff/23/?file=890575#file890575line3887 authenticated successfully is confusing. do you mean completed authentication process? Fixed by removing out promise/future alltogether. On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/authentication/cram_md5/authenticator.cpp, lines 419-421 https://reviews.apache.org/r/27760/diff/23/?file=890571#file890571line419 looks like AuthenticatorSessionProcess already has an onDiscard handler that transitions the innermost future to FAILED. Do we still need the onDiscard handler here? Yeah, I noticed that as well, now that I was re-re-refactoring things :) -- we got confused here, manifested in the previous update to this RR. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review76090 --- On March 12, 2015, 12:32 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/ --- (Updated March 12, 2015, 12:32 a.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos Description --- The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies. By introducing a two fold design, this has been decoupled from the authentication message processing. The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos. Diffs - include/mesos/authentication/authenticator.hpp f66217a src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION src/authentication/cram_md5/auxprop.hpp b894386 src/master/master.hpp 3c957ab src/master/master.cpp dccd7c6 src/tests/cram_md5_authentication_tests.cpp 92a89c5 Diff: https://reviews.apache.org/r/27760/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31960: Added concurrency protection within the SASL auxprop plugin code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/#review76177 --- Patch looks great! Reviews applied: [31960] All tests passed. - Mesos ReviewBot On March 11, 2015, 11:06 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/ --- (Updated March 11, 2015, 11:06 p.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/authentication/cram_md5/auxprop.hpp b894386 src/authentication/cram_md5/auxprop.cpp cf503a2 Diff: https://reviews.apache.org/r/31960/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31903: Added Python binding for the acceptOffers API.
On March 11, 2015, 9:38 p.m., Michael Park wrote: src/examples/python/test_framework.py, line 98 https://reviews.apache.org/r/31903/diff/1/?file=890454#file890454line98 Realized that the Java test framework has a filter here but this one doesn't. Do we want one here too? or is it more or less arbitrary? Just want to keep the existing semantics in this patch:) - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/#review76129 --- On March 10, 2015, 6:09 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/ --- (Updated March 10, 2015, 6:09 p.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2428 https://issues.apache.org/jira/browse/MESOS-2428 Repository: mesos Description --- Added Python binding for the acceptOffers API. Diffs - src/examples/python/test_framework.py 27106147900f8b5dd2ea0443f5658902ff1145e4 src/python/interface/src/mesos/interface/__init__.py f3d96a455dc8b66fc4527af1b3dee2f8841b29dd src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp a6980002202b829b40e85188bfb291e5d22c9a26 src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp bb1884597731c73f4815069ceb940cf067790670 Diff: https://reviews.apache.org/r/31903/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 31903: Added Python binding for the acceptOffers API.
On March 11, 2015, 7:30 p.m., Ben Mahler wrote: This patch looks good, but can you please include updates to the CHANGELOG and Upgrades documents in this review chain to capture this API change in 0.23.0? Added ticket https://issues.apache.org/jira/browse/MESOS-2481 to track. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/#review76110 --- On March 10, 2015, 6:09 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31903/ --- (Updated March 10, 2015, 6:09 p.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2428 https://issues.apache.org/jira/browse/MESOS-2428 Repository: mesos Description --- Added Python binding for the acceptOffers API. Diffs - src/examples/python/test_framework.py 27106147900f8b5dd2ea0443f5658902ff1145e4 src/python/interface/src/mesos/interface/__init__.py f3d96a455dc8b66fc4527af1b3dee2f8841b29dd src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp a6980002202b829b40e85188bfb291e5d22c9a26 src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp bb1884597731c73f4815069ceb940cf067790670 Diff: https://reviews.apache.org/r/31903/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 30546: MemIsolator: expose memory pressures for containers.
On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.hpp, line 22 https://reviews.apache.org/r/30546/diff/2/?file=881463#file881463line22 why is this header treated like something installed while the other slave headers are treated as local dev headers? consistency rules here.. this should be lower down and slave/isolator.hpp i believe. introduced from the following comment. commit 0082eb6d1a4cb87b2fb3672524a02d04e02854f9 Author: Kapil Arya ka...@mesosphere.io Date: Tue Feb 10 09:54:37 2015 -0800 Out of tree build 5: Exposed slave/containerizer/isolator.hpp as mesos/slave/isolator.hpp Expose slave/state.hpp and slave/containerizer/isolator.hpp to allow modules to include them directly from the Mesos install location. Review: https://reviews.apache.org/r/29603 On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 432 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line432 instead of a list, why not a vector where it is indexed by 'LOW', 'MEDIUM' and 'CRITICAL'? then in the continuation you don't have to assume the ordering in the list. list doesnt allow indexing, unfortunately. changed to use tuple. On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 436 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line436 a separate failure for each listener might help debugging down the road. fixed. On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 442 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line442 await or collect? await allows faiure, which works better with my case. On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 444 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line444 the general rule is this would be _pressureListen changed to _usage On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 668 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line668 cbegin is only in c++11 .. is it definitely in g++-4.6? avoided using iterator anymore. On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/cgroups/mem.cpp, line 672 https://reviews.apache.org/r/30546/diff/2/?file=881464#file881464line672 iterator-isReady() is clearer fixed. On Feb. 28, 2015, 1:59 a.m., Dominic Hamon wrote: src/tests/slave_recovery_tests.cpp, line 3571 https://reviews.apache.org/r/30546/diff/2/?file=881465#file881465line3571 s/ // fixed. - Chi --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/#review74647 --- On Feb. 28, 2015, 1:43 a.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/ --- (Updated Feb. 28, 2015, 1:43 a.m.) Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-2136 https://issues.apache.org/jira/browse/MESOS-2136 Repository: mesos Description --- MemIsolator: expose memory pressures for containers. Diffs - include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/slave/containerizer/isolators/cgroups/mem.hpp a00f723de61b9bccd76a2948b6d14fde7a993a8d src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e src/tests/slave_recovery_tests.cpp 53adae0118a26e6d25a9ff20c6374cc8e73275b1 Diff: https://reviews.apache.org/r/30546/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 6:27 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- Added a TODO about conflicting env vars. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
On March 11, 2015, 9:38 p.m., Ben Mahler wrote: src/master/http.cpp, line 471 https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471 Do you know if a move is helpful here or if the compiler is already optimizing this? ``` object.values[slaves] = std::move(array); ``` Don't do it in this change, we need to measure and I'm just curious :) I believe the compiler won't optimize this, since `array` is an lvalue and may be used after this assignment. One thing how we can try to persuade the compiler to move without writing `std::move()` is to wrap array population in a function that returns an array, this will be a rvalue then and c++11-capable compiler *should* choose the right overload, that takes a rvalue ref: http://www.cplusplus.com/reference/map/map/insert/. But everything more complex than `.reserve()` should be benchmarked, on all official supported compilers if possible. I'll play with this when I have some free cycles. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/#review76133 --- On March 7, 2015, 1:43 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/ --- (Updated March 7, 2015, 1:43 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2353 https://issues.apache.org/jira/browse/MESOS-2353 Repository: mesos Description --- See summary. Diffs - src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9 src/master/http.cpp b8eef69505b147d4c8a0e005dff545b9fc12a220 Diff: https://reviews.apache.org/r/31700/diff/ Testing --- make check (OS X 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 30546: MemIsolator: expose memory pressures for containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/ --- (Updated March 11, 2015, 10:32 p.m.) Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-2136 https://issues.apache.org/jira/browse/MESOS-2136 Repository: mesos Description --- MemIsolator: expose memory pressures for containers. Diffs (updated) - include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/slave/containerizer/isolators/cgroups/mem.hpp a00f723de61b9bccd76a2948b6d14fde7a993a8d src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e src/tests/slave_recovery_tests.cpp 53adae0118a26e6d25a9ff20c6374cc8e73275b1 Diff: https://reviews.apache.org/r/30546/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/ --- (Updated March 11, 2015, 10:40 p.m.) Review request for mesos and Ben Mahler. Changes --- Addressed Ben Mahler's comments. Bugs: MESOS-2353 https://issues.apache.org/jira/browse/MESOS-2353 Repository: mesos Description --- See summary. Diffs (updated) - src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9 src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24 Diff: https://reviews.apache.org/r/31700/diff/ Testing --- make check (OS X 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
On March 11, 2015, 9:38 p.m., Ben Mahler wrote: src/master/http.cpp, line 471 https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471 Do you know if a move is helpful here or if the compiler is already optimizing this? ``` object.values[slaves] = std::move(array); ``` Don't do it in this change, we need to measure and I'm just curious :) Alexander Rukletsov wrote: I believe the compiler won't optimize this, since `array` is an lvalue and may be used after this assignment. One thing how we can try to persuade the compiler to move without writing `std::move()` is to wrap array population in a function that returns an array, this will be a rvalue then and c++11-capable compiler *should* choose the right overload, that takes a rvalue ref: http://www.cplusplus.com/reference/map/map/insert/. But everything more complex than `.reserve()` should be benchmarked, on all official supported compilers if possible. I'll play with this when I have some free cycles. From writing my own JSON code, the std::move here is definitely necessary to do this as quickly and cheaply as possible. Using std::move here I think is actually the right thing to do, rather than trying to convince the compiler via other means, it's explicitly saying to the reader This variable you were using earlier, it is having it's contents ripped out of it. Putting it into a function and guaranteeing NRVO or RVO fire so that you get the cheap move insertion has a lot of things people can disturb only slightly and end up breaking it. - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/#review76133 --- On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31700/ --- (Updated March 11, 2015, 10:40 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2353 https://issues.apache.org/jira/browse/MESOS-2353 Repository: mesos Description --- See summary. Diffs - src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9 src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24 Diff: https://reviews.apache.org/r/31700/diff/ Testing --- make check (OS X 10.9.5, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 31960: Added concurrency protection within the SASL auxprop plugin code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/#review76164 --- Ship it! Ship It! - Vinod Kone On March 11, 2015, 11:06 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/ --- (Updated March 11, 2015, 11:06 p.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/authentication/cram_md5/auxprop.hpp b894386 src/authentication/cram_md5/auxprop.cpp cf503a2 Diff: https://reviews.apache.org/r/31960/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 7:42 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Changes --- minor grammer fix. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs (updated) - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Review Request 31961: Split cram_md5 authenticator into declaration and implementation without functional changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31961/ --- Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/Makefile.am 3059818 src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION Diff: https://reviews.apache.org/r/31961/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31961: Split cram_md5 authenticator into declaration and implementation without functional changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31961/#review76176 --- Ship it! Thanks for the split! - Vinod Kone On March 12, 2015, 12:02 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31961/ --- (Updated March 12, 2015, 12:02 a.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/Makefile.am 3059818 src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION Diff: https://reviews.apache.org/r/31961/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 30545: cgroups: added support to listen on memory pressures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated March 11, 2015, 10:30 p.m.) Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-2136 https://issues.apache.org/jira/browse/MESOS-2136 Repository: mesos Description --- cgroups: added support to listen on memory pressures. Diffs (updated) - src/linux/cgroups.hpp e07772fec11b9bb73a44c54326f24d7ee1e8a64b src/linux/cgroups.cpp a533b319fc75abb0fc45b8f5f473f257912d21ac Diff: https://reviews.apache.org/r/30545/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 3:30 p.m., Adam B wrote: src/slave/containerizer/containerizer.cpp, lines 295-302 https://reviews.apache.org/r/31889/diff/9/?file=891672#file891672line295 What about other env[] variables set earlier in this function? If the decorator wants to check/modify MESOS_DIRECTORY or other MESOS_foo env variables, it won't know what they are being set to and will have to blindly override them if it wants to set them. Maybe we could pass a mutable copy of ExecutorInfo where the env[] variables have been merged in, or go back to passing env[] itself, after the executorInfo defaults have been set. A nice to have for now IMO. We can leave a comment/TODO/JIRA but I think we should discuss whether we actually want the decorator to change internal environment variables. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76147 --- On March 11, 2015, 3:27 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 3:27 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31873: Added Java binding for the new acceptOffers API.
On March 11, 2015, 7:14 p.m., Ben Mahler wrote: src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp, lines 902-903 https://reviews.apache.org/r/31873/diff/1/?file=889870#file889870line902 Want to collapse these lines to just `push_back(construct...(...));` Done. On March 11, 2015, 7:14 p.m., Ben Mahler wrote: src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp, lines 925-928 https://reviews.apache.org/r/31873/diff/1/?file=889870#file889870line925 Want to collapse these lines to just `push_back(construct...(...))`? Done. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/#review76106 --- On March 10, 2015, 12:07 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/ --- (Updated March 10, 2015, 12:07 a.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2427 https://issues.apache.org/jira/browse/MESOS-2427 Repository: mesos Description --- Added Java binding for the new acceptOffers API. Diffs - src/examples/java/TestFramework.java 65ba9d9e242f40d6382cd9c18269b87f8b59b5a1 src/java/jni/construct.cpp e54c11ef0ce3e9f6d0e572d3a89cadae417f9cd6 src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 4f0dad78f7300b7bbf147f231eb053fd7faf4b69 src/java/src/org/apache/mesos/MesosSchedulerDriver.java a1055a5d907133485891ebd6d8731c102b913fec src/java/src/org/apache/mesos/SchedulerDriver.java d5b100a4c371bd9c496b9127767c14047185e5f9 Diff: https://reviews.apache.org/r/31873/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76142 --- Ship it! Ship It! - Niklas Nielsen On March 11, 2015, 2:54 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 2:54 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 7:06 p.m., Adam B wrote: src/slave/containerizer/containerizer.cpp, lines 294-297 https://reviews.apache.org/r/31889/diff/9-11/?file=891672#file891672line294 Not sure this TODO is accurate/necessary anymore. This patch assumes that the decorator hook returns the entire env from executorInfo as well as its own modifications, so it's up to the hook to decide how to resolve conflicts. On the other hand, if the hook incorrectly returns only its own environment, then the executorInfo.environment variables will be silently dropped. The hook still returns only it's own environment variables. The hook manager merges the returned values with the executorInfo's environment and finally returns an aggregate. However, the caller can still overwrite anything in the aggregated result by overwriting with the values from the original executorInfo's environment. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76158 --- On March 11, 2015, 6:51 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 6:51 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
On March 11, 2015, 9:38 p.m., Ben Mahler wrote:
src/master/http.cpp, line 471
https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471
Do you know if a move is helpful here or if the compiler is already
optimizing this?
```
object.values[slaves] = std::move(array);
```
Don't do it in this change, we need to measure and I'm just curious :)
Alexander Rukletsov wrote:
I believe the compiler won't optimize this, since `array` is an lvalue
and may be used after this assignment. One thing how we can try to persuade
the compiler to move without writing `std::move()` is to wrap array
population in a function that returns an array, this will be a rvalue then
and c++11-capable compiler *should* choose the right overload, that takes a
rvalue ref: http://www.cplusplus.com/reference/map/map/insert/.
But everything more complex than `.reserve()` should be benchmarked, on
all official supported compilers if possible. I'll play with this when I have
some free cycles.
Cody Maloney wrote:
From writing my own JSON code, the std::move here is definitely necessary
to do this as quickly and cheaply as possible.
Using std::move here I think is actually the right thing to do, rather
than trying to convince the compiler via other means, it's explicitly saying
to the reader This variable you were using earlier, it is having it's
contents ripped out of it.
Putting it into a function and guaranteeing NRVO or RVO fire so that you
get the cheap move insertion has a lot of things people can disturb only
slightly and end up breaking it.
Cody Maloney wrote:
Updating the instance in stout/protobuf.hpp where the JSON::Array is
copied to be a move would also probably help significantly.
The way JSON::Protobuf's api is setup with Operator() forces a copy of
the JSON::Object after it is fully constructed which is rather expensive.
Would be good to make JSON::Protobuf() just be a function which can then use
the object internally, and move out the result rather than forcing the full
object copy.
Michael Park wrote:
TL;DR: `std::move` will help.
### Does the compiler see that the `array` variable can be moved? No.
It looks like the compiler could probably perform a NRVO-like
optimization here, but it doesn't.
```cpp
#include iostream
class Foo {
public:
Foo() { std::cout Foo() std::endl; }
Foo(const Foo ) { std::cout Foo(const Foo ) std::endl; }
Foo(Foo ) noexcept { std::cout Foo(Foo ) std::endl; }
Foo operator=(const Foo ) {
std::cout Foo operator=(const Foo ) std::endl;
return *this;
}
Foo operator=(Foo ) noexcept {
std::cout Foo operator=(Foo ) std::endl;
return *this;
}
};
int main() {
Foo result;
{
Foo x;
result = x;
}
{
Foo y;
result = std::move(y);
}
}
```
The above prints:
```bash
Foo()
Foo()
Foo operator=(const Foo )
Foo()
Foo operator=(Foo )
```
That is compiled with `clang++ -std=c++14 -O2 a.cc`. It's safe to say
that if can't optimize that obvious-looking case, ours won't be optimized
either.
The reason why I mentioned convincing the compiler instead of `std::move()`, is
because I don't see `std::move()` in
https://mesos.apache.org/documentation/latest/mesos-c++-style-guide/
- Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/#review76133
---
On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/
---
(Updated March 11, 2015, 10:40 p.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-2353
https://issues.apache.org/jira/browse/MESOS-2353
Repository: mesos
Description
---
See summary.
Diffs
-
src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9
src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24
Diff: https://reviews.apache.org/r/31700/diff/
Testing
---
make check (OS X 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31324: Updated changelog for 0.22.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/ --- (Updated March 11, 2015, 5:28 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Changes --- Addressed Adams comments Repository: mesos Description --- Updated changelog for Mesos 0.22.0 Diffs (updated) - CHANGELOG 2a54f08b7bb9cc6983631268eafec8cb7d166d97 Diff: https://reviews.apache.org/r/31324/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 31324: Updated changelog for 0.22.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/ --- (Updated March 11, 2015, 5:27 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Repository: mesos Description --- Updated changelog for Mesos 0.22.0 Diffs (updated) - CHANGELOG 2a54f08b7bb9cc6983631268eafec8cb7d166d97 Diff: https://reviews.apache.org/r/31324/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 31873: Added Java binding for the new acceptOffers API.
On March 11, 2015, 9:19 p.m., Michael Park wrote: The Java changes look good to me. I've only skimmed over the C++ JNI changes, JNI code is... painful. Look give a second sweep over the JNI code soon. Meanwhile, just a few nits on comments. I'll commit those first. Let me know if you have further comments and I'll address them. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/#review76128 --- On March 10, 2015, 12:07 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31873/ --- (Updated March 10, 2015, 12:07 a.m.) Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. Bugs: MESOS-2427 https://issues.apache.org/jira/browse/MESOS-2427 Repository: mesos Description --- Added Java binding for the new acceptOffers API. Diffs - src/examples/java/TestFramework.java 65ba9d9e242f40d6382cd9c18269b87f8b59b5a1 src/java/jni/construct.cpp e54c11ef0ce3e9f6d0e572d3a89cadae417f9cd6 src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 4f0dad78f7300b7bbf147f231eb053fd7faf4b69 src/java/src/org/apache/mesos/MesosSchedulerDriver.java a1055a5d907133485891ebd6d8731c102b913fec src/java/src/org/apache/mesos/SchedulerDriver.java d5b100a4c371bd9c496b9127767c14047185e5f9 Diff: https://reviews.apache.org/r/31873/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 31324: Updated changelog for 0.22.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/#review76188 --- Patch looks great! Reviews applied: [31324] All tests passed. - Mesos ReviewBot On March 12, 2015, 12:28 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/ --- (Updated March 12, 2015, 12:28 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Repository: mesos Description --- Updated changelog for Mesos 0.22.0 Diffs - CHANGELOG 2a54f08b7bb9cc6983631268eafec8cb7d166d97 Diff: https://reviews.apache.org/r/31324/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76049 --- Looks good. Just a couple of questions and some style nits in the new test. src/hook/manager.cpp https://reviews.apache.org/r/31889/#comment123441 What should you do if result.isNone()? Just ignore without warning (as you're already doing here)? src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123448 Does this being a HookTest mean that it's guaranteed to use the FOO=bar test executorEnvironmentDecorator hook? src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123442 src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123443 src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123444 .Times(1) is implicit, can be removed. src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123445 src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123449 Where do you actually validate the return value here? Is it just that you're expecting TASK_FINISHED instead of TASK_FAILED? src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123446 s/MergeFrom/CopyFrom/g src/tests/hook_tests.cpp https://reviews.apache.org/r/31889/#comment123447 Could use the CREATE_EXECUTOR_INFO() + LaunchTasks() model instead of building up a TaskInfo yourself. - Adam B On March 10, 2015, 7:15 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 10, 2015, 7:15 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 8faf6850aafcebda7e9f0d1b735d61f7effa842d src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 364d911b086dfe1f15f76aa3888f99146aa8d876 src/tests/hook_tests.cpp f4b4f519456dc00a8894c7ce154b28a7ab9ce493 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 30774: Fetcher Cache
On March 9, 2015, 8:37 a.m., Joerg Schad wrote: include/mesos/mesos.proto, line 208 https://reviews.apache.org/r/30774/diff/33/?file=888347#file888347line208 Could you add a comment (i.e. backlink to the documention) reminding developers to update docs/fetcher.md when the protobuf is changed? Since we are dropping the enum, there will be no such comment. There is one next to the remaining cache filed, though. On March 9, 2015, 8:37 a.m., Joerg Schad wrote: src/slave/containerizer/fetcher.cpp, line 450 https://reviews.apache.org/r/30774/diff/33/?file=888355#file888355line450 size_t position? Using an iterator now. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75700 --- On March 7, 2015, 7:21 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 7, 2015, 7:21 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only one download. The fetcher program remains external for safety reasons, but is now augmented with more elaborate parameters packed into a JSON object to implement specific fetch actions for all of the above. Additional testing includes fetching from (mock) HDFS and coverage of the new features. Diffs - docs/fetcher-cache-internals.md PRE-CREATION docs/fetcher.md PRE-CREATION include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp ec4626f903d44c0911093ff763ef16ad27c418a9 src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 src/slave/slave.cpp a06d68032f26ccb3f786b6ea7c3a6c3c52449bd2 src/tests/docker_containerizer_tests.cpp 06cd3d89ecbaaac17ae6970604b21fbe29f6e887 src/tests/fetcher_cache_tests.cpp PRE-CREATION src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e src/tests/mesos.hpp e91e5e484eea4587ac8f2eb9cefeab4acc9f4615 src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1 Diff: https://reviews.apache.org/r/30774/diff/ Testing --- make check --- longer Description: --- -Replaces all other reviews for the fetcher cache except those related to stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 30618, 30621, 30626. See descriptions of those. In dependency order: 30033: Removes the fetcher env tests since these won't be needed any more when the fetcher uses JSON in a single env var as a parameter. They never tested anything that won't be covered by other tests anyway. 30034: Makes the code structure of all fetcher tests the same. Instead of calling the run method of the fetcher directly, calling through fetch(). Also removes all uses of I/O redirection, which is not really needed for debugging, and thus the next patch can refactor fetch() and run(). (The latter comes in two varieties, which complicates matters without much benefit.) 30036:
Re: Review Request 30774: Fetcher Cache
On March 6, 2015, 2:15 p.m., Timothy Chen wrote: include/mesos/fetcher/fetcher.proto, line 58 https://reviews.apache.org/r/30774/diff/32/?file=887350#file887350line58 It's harder to make a optional field required, but it's much easier the other way around. If we always want it to be required, I think we should make the sandbox a required field. Bernd Mathiske wrote: There was some discussion about whether this field should be required or not. The general idea here is that a task might be able to run without fetching anything into its sandbox. In this case, the framework may get away without naming the sandbox. But since a task always has one, we could also make it required. I am impartial in this choice, but I see that your argument that required-optional is easier has pull. I have heard good arguments both ways. Here is how I see it. For the recipient of a message, optional is the preferred choice. Then any legacy recipient's code is always prepared for everything and robust wrt. changing to required. Not the other way around. But for the sender, required is the better choice, making sender code more robust. If legacy senders still provide the field when it has become optional, that's OK. Not the other way around. So which side are we on in this case? As much as this is an internal protocol, we are on neither side and we can change this in arbitrary ways. This is an external protocol if someone else than a Mesos slave uses mesos-fetcher. (Maybe a special external containerizer.) Then we are providing the message recipient and we have to be on that side. Therefore I am voting for optional. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75491 --- On March 7, 2015, 7:21 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 7, 2015, 7:21 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only one download. The fetcher program remains external for safety reasons, but is now augmented with more elaborate parameters packed into a JSON object to implement specific fetch actions for all of the above. Additional testing includes fetching from (mock) HDFS and coverage of the new features. Diffs - docs/fetcher-cache-internals.md PRE-CREATION docs/fetcher.md PRE-CREATION include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp ec4626f903d44c0911093ff763ef16ad27c418a9 src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 src/slave/slave.cpp a06d68032f26ccb3f786b6ea7c3a6c3c52449bd2 src/tests/docker_containerizer_tests.cpp 06cd3d89ecbaaac17ae6970604b21fbe29f6e887 src/tests/fetcher_cache_tests.cpp PRE-CREATION src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e src/tests/mesos.hpp
Re: Review Request 30774: Fetcher Cache
On March 2, 2015, 8:13 p.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 759 https://reviews.apache.org/r/30774/diff/25/?file=882228#file882228line759 If you're incrementing all the time just to count, why not just get the size from list? Bernd Mathiske wrote: I am not incrementing to count anything. I am incrementing to hit the right index in a vector that parallels the list I am iterating over. Is there a C++ or Boost construct that can do this without indices? Switched to using a const_iterator for this. This should be more obviously paralleling the foreach. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74885 --- On March 7, 2015, 7:21 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 7, 2015, 7:21 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only one download. The fetcher program remains external for safety reasons, but is now augmented with more elaborate parameters packed into a JSON object to implement specific fetch actions for all of the above. Additional testing includes fetching from (mock) HDFS and coverage of the new features. Diffs - docs/fetcher-cache-internals.md PRE-CREATION docs/fetcher.md PRE-CREATION include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp ec4626f903d44c0911093ff763ef16ad27c418a9 src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 src/slave/slave.cpp a06d68032f26ccb3f786b6ea7c3a6c3c52449bd2 src/tests/docker_containerizer_tests.cpp 06cd3d89ecbaaac17ae6970604b21fbe29f6e887 src/tests/fetcher_cache_tests.cpp PRE-CREATION src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e src/tests/mesos.hpp e91e5e484eea4587ac8f2eb9cefeab4acc9f4615 src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1 Diff: https://reviews.apache.org/r/30774/diff/ Testing --- make check --- longer Description: --- -Replaces all other reviews for the fetcher cache except those related to stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 30618, 30621, 30626. See descriptions of those. In dependency order: 30033: Removes the fetcher env tests since these won't be needed any more when the fetcher uses JSON in a single env var as a parameter. They never tested anything that won't be covered by other tests anyway. 30034: Makes the code structure of all fetcher tests the same. Instead of calling the run method of the fetcher directly, calling through fetch(). Also removes all uses of I/O redirection, which is not really needed for debugging, and thus the next patch can refactor fetch() and run(). (The latter comes in two varieties, which complicates matters without much benefit.) 30036: Extends the CommandInfo::URI protobuf with
Re: Review Request 30774: Fetcher Cache
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 11, 2015, 5:47 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Changes --- Addressed almost all issues. Simplified fetcher cache test source code. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only one download. The fetcher program remains external for safety reasons, but is now augmented with more elaborate parameters packed into a JSON object to implement specific fetch actions for all of the above. Additional testing includes fetching from (mock) HDFS and coverage of the new features. Diffs (updated) - docs/configuration.md fc3afec248b534b1d5eb625eb66de5f90cd8cd33 docs/fetcher-cache-internals.md PRE-CREATION docs/fetcher.md PRE-CREATION include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 src/Makefile.am d299f07d865080676ca8a550cf6005c6ab32839f src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp ec4626f903d44c0911093ff763ef16ad27c418a9 src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 src/slave/slave.cpp a06d68032f26ccb3f786b6ea7c3a6c3c52449bd2 src/tests/docker_containerizer_tests.cpp 06cd3d89ecbaaac17ae6970604b21fbe29f6e887 src/tests/fetcher_cache_tests.cpp PRE-CREATION src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e src/tests/mesos.hpp e91e5e484eea4587ac8f2eb9cefeab4acc9f4615 src/tests/mesos.cpp c8f43d21b214e75eaac2870cbdf4f03fd18707d1 Diff: https://reviews.apache.org/r/30774/diff/ Testing --- make check --- longer Description: --- -Replaces all other reviews for the fetcher cache except those related to stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 30618, 30621, 30626. See descriptions of those. In dependency order: 30033: Removes the fetcher env tests since these won't be needed any more when the fetcher uses JSON in a single env var as a parameter. They never tested anything that won't be covered by other tests anyway. 30034: Makes the code structure of all fetcher tests the same. Instead of calling the run method of the fetcher directly, calling through fetch(). Also removes all uses of I/O redirection, which is not really needed for debugging, and thus the next patch can refactor fetch() and run(). (The latter comes in two varieties, which complicates matters without much benefit.) 30036: Extends the CommandInfo::URI protobuf with a boolean caching field that will later cause fetcher cache actions. Also introduces the notion of a cache directory to the fetcher info protobuf. And then propagates these additions throughout the rest of the code base where applicable. This includes passing the slave ID all the way down to the place where the cache dir name is constructed. 30037: Extends the fetcher info protobuf with actions (fetch directly bypassing the cache, fetch through the cache, retrieve from the cache). Switches the basis for dealing with uris to items, which contain the uri, the action, and potentially a cache file name. Refactors fetch() and run(), so there is only one of each. Introduces about half of the actual cache logic, including a hashmap of cache file objects for bookkeeping and basic operations on it. 30039: Enables fetcher cache
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
On March 11, 2015, 4:02 a.m., Adam B wrote: src/hook/manager.cpp, line 137 https://reviews.apache.org/r/31889/diff/5-6/?file=890977#file890977line137 What should you do if result.isNone()? Just ignore without warning (as you're already doing here)? Yes, the hook decided to not add any new labels to the TaskInfo and that should be perfectly legit. That's why the Result type and not a Try type. On March 11, 2015, 4:02 a.m., Adam B wrote: src/tests/hook_tests.cpp, line 199 https://reviews.apache.org/r/31889/diff/6/?file=891026#file891026line199 Does this being a HookTest mean that it's guaranteed to use the FOO=bar test executorEnvironmentDecorator hook? I am not sure if I understand your question here. The test hook sets FOO=bar in the slaveExecutorEnvironmentDecorator and that's what we verify here. This test (along with other tests) is tightly coupled with the test hook module. On March 11, 2015, 4:02 a.m., Adam B wrote: src/tests/hook_tests.cpp, line 234 https://reviews.apache.org/r/31889/diff/6/?file=891026#file891026line234 Where do you actually validate the return value here? Is it just that you're expecting TASK_FINISHED instead of TASK_FAILED? Yes. I verified the result with test $FOO = 'bbar' and it results in TASK_FAILED instead of TASK_FINISHED. On March 11, 2015, 4:02 a.m., Adam B wrote: src/tests/hook_tests.cpp, line 253 https://reviews.apache.org/r/31889/diff/6/?file=891026#file891026line253 Could use the CREATE_EXECUTOR_INFO() + LaunchTasks() model instead of building up a TaskInfo yourself. Good point. Will update the diffs. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76049 --- On March 10, 2015, 10:15 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 10, 2015, 10:15 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 8faf6850aafcebda7e9f0d1b735d61f7effa842d src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 364d911b086dfe1f15f76aa3888f99146aa8d876 src/tests/hook_tests.cpp f4b4f519456dc00a8894c7ce154b28a7ab9ce493 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. The master failover tests have not been performed yet. Thanks, Kapil Arya
Re: Review Request 30774: Fetcher Cache
On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/launcher/fetcher.cpp, line 321 https://reviews.apache.org/r/30774/diff/33/?file=888350#file888350line321 Can you comment the relationship between the FetcherInfo::Item and the FetcherInfo here? Is the FetcherInfo::Item within the FetcherInfo but FetcherInfo is included because you just want to get the 'sandbox_directory' and 'cache_directory' and rather than pulling those out explicitly you just passed the entire FetcherInfo? There are more items in the FetcherInfo than just the one we are working in here. That's why this one is called out explicitly. I changed this to passing both directories in. On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/launcher/fetcher.cpp, lines 364-366 https://reviews.apache.org/r/30774/diff/33/?file=888350#file888350line364 Why are these not CHECKs? Since you're the one setting up the FetcherInfo it seems like you should know explicitly whether or not the cache_filename was set! Same for the cache_directory below as well. What if somebody else uses mesos-fetcher? On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/launcher/fetcher.cpp, lines 403-404 https://reviews.apache.org/r/30774/diff/33/?file=888350#file888350line403 As mentioned above, it would be great to really capture the relationship between the FetcherInfo and the FetcherInfo::Item. If The FetcherInfo encapsulates the FetcherInfo::Item I would also suggest switching the order of the parameters to signify that. The main purpose here is to fetch this one particular item, not everything FetcherInfo carries. FetcherInfo is a secondary parameter that provides extra parameters like cache_directory, sandbox_directory, and framework_home. Putting it second makes this relationship clear IMHO. Do you suggest adding all these as individual parameters? Yes, the item is included in the list of items in FetcherInfo. Shall we break up FetcherInfo into several shells, the inner one without items? On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/slave/flags.hpp, line 487 https://reviews.apache.org/r/30774/diff/33/?file=888358#file888358line487 Can we make this a Path to start? Then it would be the only one. Confusing. I'd rather have a wholesale sweep over the whole code base to introduce Path - as a separate ticket. On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/slave/slave.cpp, line 796 https://reviews.apache.org/r/30774/diff/33/?file=888359#file888359line796 We should do recovery on the fetcher itself: TryNothing recover = fetcher-recover(flags, slaveId); It seems very weird to have a static generic Fetcher recover functionality that implies that we can't have multiple Fetchers running at the same time. How do we start multiple slaves at the same time? This is an artefact of the lack of injection of slaveId and flags. It should be cleaned up when we refactor those. The slave does not have access to the fetcher instance as it is right now. It would cause a lot of collateral changes if it did. I advise to refrain for now. I have put a comment at the static method to explain this. That's the best fix for now IMHO. There is no problem starting multiple slaves, because they all have a different slaveID that gets passed into this call. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75754 --- On March 7, 2015, 7:21 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 7, 2015, 7:21 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074 https://issues.apache.org/jira/browse/MESOS-2057 https://issues.apache.org/jira/browse/MESOS-2069 https://issues.apache.org/jira/browse/MESOS-2070 https://issues.apache.org/jira/browse/MESOS-2072 https://issues.apache.org/jira/browse/MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2074 Repository: mesos Description --- Almost all of the functionality in epic MESOS-336. Downloaded files from CommandInfo::URIs can now be cached in a cache directory designated by a slave flag. This only happens when asked for by an extra flag in the URI and is thus backwards-compatible. The cache has a size limit also given by a new slave flag. Cache-resident files are evicted as necessary to make space for newly fetched ones. Concurrent attempts to cache the same URI leads to only
Re: Review Request 31324: Updated changelog for 0.22.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/#review76203 --- Ship it! Looks more or less good to go. I just want to call your attention to a few features that could use references to further documentation, and a couple of fixes/features that might warrant further note. CHANGELOG https://reviews.apache.org/r/31324/#comment123681 Any doc to point to with more info? CHANGELOG https://reviews.apache.org/r/31324/#comment123682 Again, doc reference? s/;/:/ CHANGELOG https://reviews.apache.org/r/31324/#comment123683 Also in ExecutorInfo, but that's probably mentioned in the framework development guide. CHANGELOG https://reviews.apache.org/r/31324/#comment123684 Where should users look for this information now? The JIRA leads me to believe '/metrics/snapshot', but we need to have a clear answer. Is there further documentation on the metrics endpoints? CHANGELOG https://reviews.apache.org/r/31324/#comment123685 Possibly an API change, since custom resource types will now show up in slave's state.json, but not likely to break anything. CHANGELOG https://reviews.apache.org/r/31324/#comment123686 Another potential API change/fix, for reconciliation status updates. This one feels even less likely to surprise anybody CHANGELOG https://reviews.apache.org/r/31324/#comment123687 Do we want to call out libevent support as a new feature? Or go silent until SSL can be enabled? CHANGELOG https://reviews.apache.org/r/31324/#comment123689 Are we leaving DiskInfo (Persistent Volumes) in silent mode as well, presumably due to a few missing pieces? CHANGELOG https://reviews.apache.org/r/31324/#comment123688 Similarly, do we want to call out the new Accept(offerIds, Operation) API, or wait until DiskInfo/DynamicReservations are ready before we try and push people off of the old LaunchTasks API? - Adam B On March 11, 2015, 5:28 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/ --- (Updated March 11, 2015, 5:28 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Repository: mesos Description --- Updated changelog for Mesos 0.22.0 Diffs - CHANGELOG 2a54f08b7bb9cc6983631268eafec8cb7d166d97 Diff: https://reviews.apache.org/r/31324/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 31700: Reserved memory for JSON arrays where appropriate.
On March 11, 2015, 9:38 p.m., Ben Mahler wrote:
src/master/http.cpp, line 471
https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471
Do you know if a move is helpful here or if the compiler is already
optimizing this?
```
object.values[slaves] = std::move(array);
```
Don't do it in this change, we need to measure and I'm just curious :)
Alexander Rukletsov wrote:
I believe the compiler won't optimize this, since `array` is an lvalue
and may be used after this assignment. One thing how we can try to persuade
the compiler to move without writing `std::move()` is to wrap array
population in a function that returns an array, this will be a rvalue then
and c++11-capable compiler *should* choose the right overload, that takes a
rvalue ref: http://www.cplusplus.com/reference/map/map/insert/.
But everything more complex than `.reserve()` should be benchmarked, on
all official supported compilers if possible. I'll play with this when I have
some free cycles.
Cody Maloney wrote:
From writing my own JSON code, the std::move here is definitely necessary
to do this as quickly and cheaply as possible.
Using std::move here I think is actually the right thing to do, rather
than trying to convince the compiler via other means, it's explicitly saying
to the reader This variable you were using earlier, it is having it's
contents ripped out of it.
Putting it into a function and guaranteeing NRVO or RVO fire so that you
get the cheap move insertion has a lot of things people can disturb only
slightly and end up breaking it.
Cody Maloney wrote:
Updating the instance in stout/protobuf.hpp where the JSON::Array is
copied to be a move would also probably help significantly.
The way JSON::Protobuf's api is setup with Operator() forces a copy of
the JSON::Object after it is fully constructed which is rather expensive.
Would be good to make JSON::Protobuf() just be a function which can then use
the object internally, and move out the result rather than forcing the full
object copy.
TL;DR: `std::move` will help.
### Does the compiler see that the `array` variable can be moved? No.
It looks like the compiler could probably perform a NRVO-like optimization
here, but it doesn't.
```cpp
#include iostream
class Foo {
public:
Foo() { std::cout Foo() std::endl; }
Foo(const Foo ) { std::cout Foo(const Foo ) std::endl; }
Foo(Foo ) noexcept { std::cout Foo(Foo ) std::endl; }
Foo operator=(const Foo ) {
std::cout Foo operator=(const Foo ) std::endl;
return *this;
}
Foo operator=(Foo ) noexcept {
std::cout Foo operator=(Foo ) std::endl;
return *this;
}
};
int main() {
Foo result;
{
Foo x;
result = x;
}
{
Foo y;
result = std::move(y);
}
}
```
The above prints:
```bash
Foo()
Foo()
Foo operator=(const Foo )
Foo()
Foo operator=(Foo )
```
That is compiled with `clang++ -std=c++14 -O2 a.cc`. It's safe to say that if
can't optimize that obvious-looking case, ours won't be optimized either.
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/#review76133
---
On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/
---
(Updated March 11, 2015, 10:40 p.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-2353
https://issues.apache.org/jira/browse/MESOS-2353
Repository: mesos
Description
---
See summary.
Diffs
-
src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9
src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24
Diff: https://reviews.apache.org/r/31700/diff/
Testing
---
make check (OS X 10.9.5, Ubuntu 14.04)
Thanks,
Alexander Rukletsov
Re: Review Request 31889: Updated Hooks to fix failure during master failover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76158 --- More TODO comment cleanup src/slave/containerizer/containerizer.cpp https://reviews.apache.org/r/31889/#comment123641 Not sure this TODO is accurate/necessary anymore. This patch assumes that the decorator hook returns the entire env from executorInfo as well as its own modifications, so it's up to the hook to decide how to resolve conflicts. On the other hand, if the hook incorrectly returns only its own environment, then the executorInfo.environment variables will be silently dropped. src/slave/containerizer/containerizer.cpp https://reviews.apache.org/r/31889/#comment123637 Redundant to pass/to be passed. Either s/to pass/for/ or s/to be passed// - Adam B On March 11, 2015, 3:51 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/ --- (Updated March 11, 2015, 3:51 p.m.) Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till Toenshoff. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- The ExecutorInfo is immutable, and so the environment decoration is deferred until containerizer launch. The new slaveExecutorHook simply notifies the hook module of a new executor being launched. The environment decorator hook for containerizer launch update it's environment variables as gathered from the hook modules. Diffs - include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 src/slave/containerizer/containerizer.cpp 33f8738fa229b42ebacfd75b762c977b33191b65 src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 Diff: https://reviews.apache.org/r/31889/diff/ Testing --- make check. Also tried a negative test by replacing `test $FOO = 'bar'` with `test $FOO = 'baz'` and surely, the test failed. Thanks, Kapil Arya
Re: Review Request 31324: Updated changelog for 0.22.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/#review76183 --- LGTM. - Michael Park On March 12, 2015, 12:28 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31324/ --- (Updated March 12, 2015, 12:28 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. Repository: mesos Description --- Updated changelog for Mesos 0.22.0 Diffs - CHANGELOG 2a54f08b7bb9cc6983631268eafec8cb7d166d97 Diff: https://reviews.apache.org/r/31324/diff/ Testing --- Thanks, Niklas Nielsen
Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
On March 11, 2015, 9:42 p.m., Vinod Kone wrote: src/master/master.cpp, lines 3858-3861 https://reviews.apache.org/r/27760/diff/23/?file=890575#file890575line3858 Send a FrameworkError message (instead of AuthenticationError) here to avoid retries? The `AuthenticationErrorMessage` is sent to the AuthenticateeProcess, not the process to be authenticated (slave / framework). The behaviour on this specific event (failed to init the authenticator) has not changed. I believe we commented on this before and agreed to come up with a cleanup on authentication result handling later, in a follow-up-patch. Do I remember correctly here? - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review76090 --- On March 12, 2015, 12:32 a.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/ --- (Updated March 12, 2015, 12:32 a.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos Description --- The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies. By introducing a two fold design, this has been decoupled from the authentication message processing. The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos. Diffs - include/mesos/authentication/authenticator.hpp f66217a src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION src/authentication/cram_md5/auxprop.hpp b894386 src/master/master.hpp 3c957ab src/master/master.cpp dccd7c6 src/tests/cram_md5_authentication_tests.cpp 92a89c5 Diff: https://reviews.apache.org/r/27760/diff/ Testing --- make check Thanks, Till Toenshoff
Review Request 31930: Introduced an http::Pipe abstraction to simplify streaming HTTP Responses.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31930/ --- Review request for mesos, Benjamin Hindman and Jie Yu. Bugs: MESOS-2438 https://issues.apache.org/jira/browse/MESOS-2438 Repository: mesos Description --- See MESOS-2438 for the motivation. Diffs - 3rdparty/libprocess/include/process/http.hpp 10143fdbd5eb43f968c41957a2335f96a097d82e 3rdparty/libprocess/src/http.cpp 7c0cee404645e1dd1a9253f85883aefa29a5f999 3rdparty/libprocess/src/process.cpp e7b029ba97e640c2102548c190ba62b30602f43d 3rdparty/libprocess/src/tests/http_tests.cpp 800752a57230d7768d3d741fef87f6283757e424 Diff: https://reviews.apache.org/r/31930/diff/ Testing --- * added tests * make check Thanks, Ben Mahler
Re: Review Request 31961: Split cram_md5 authenticator into declaration and implementation without functional changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31961/#review76205 --- Ship it! LGTM - Adam B On March 11, 2015, 5:02 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31961/ --- (Updated March 11, 2015, 5:02 p.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/Makefile.am 3059818 src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION Diff: https://reviews.apache.org/r/31961/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31960: Added concurrency protection within the SASL auxprop plugin code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/#review76206 --- Ship it! Ship It! - Adam B On March 11, 2015, 4:06 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31960/ --- (Updated March 11, 2015, 4:06 p.m.) Review request for mesos, Adam B and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos-incubating Description --- see summary. Diffs - src/authentication/cram_md5/auxprop.hpp b894386 src/authentication/cram_md5/auxprop.cpp cf503a2 Diff: https://reviews.apache.org/r/31960/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
On March 11, 2015, 2:42 p.m., Vinod Kone wrote: src/master/master.cpp, lines 3858-3861 https://reviews.apache.org/r/27760/diff/23/?file=890575#file890575line3858 Send a FrameworkError message (instead of AuthenticationError) here to avoid retries? Till Toenshoff wrote: The `AuthenticationErrorMessage` is sent to the AuthenticateeProcess, not the process to be authenticated (slave / framework). The behaviour on this specific event (failed to init the authenticator) has not changed. I believe we commented on this before and agreed to come up with a cleanup on authentication result handling later, in a follow-up-patch. Do I remember correctly here? Let's create a separate JIRA so that authenticatees (or the owning framework/slave) can know from the authentication response (error/failed) whether to quit or retry. - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review76090 --- On March 11, 2015, 5:32 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/ --- (Updated March 11, 2015, 5:32 p.m.) Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2050 https://issues.apache.org/jira/browse/MESOS-2050 Repository: mesos Description --- The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies. By introducing a two fold design, this has been decoupled from the authentication message processing. The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos. Diffs - include/mesos/authentication/authenticator.hpp f66217a src/authentication/cram_md5/authenticator.hpp c6f465f src/authentication/cram_md5/authenticator.cpp PRE-CREATION src/authentication/cram_md5/auxprop.hpp b894386 src/master/master.hpp 3c957ab src/master/master.cpp dccd7c6 src/tests/cram_md5_authentication_tests.cpp 92a89c5 Diff: https://reviews.apache.org/r/27760/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 31503: Add classid to Filter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31503/#review76195 --- Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work! - Jie Yu On March 10, 2015, 11:26 p.m., Cong Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31503/ --- (Updated March 10, 2015, 11:26 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Bugs: MESOS-2422 https://issues.apache.org/jira/browse/MESOS-2422 Repository: mesos Description --- Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery. Diffs - src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba Diff: https://reviews.apache.org/r/31503/diff/ Testing --- Run the testcase. Thanks, Cong Wang
Re: Review Request 31503: Add classid to Filter
On March 12, 2015, 2:02 a.m., Jie Yu wrote: Cong, I addressed the comments for you and try to commit. Looks like rtnl_u32_get_classid is not in libnl-3.2.25. That means we need a newer version of libnl for this to work! I have addressed your comments locally, just haven't uploaded a new version. Yes, we need this commit from upstream: https://github.com/thom311/libnl/commit/d8f080d94fa9cf5e977f8805446ac7ef39f82d31 - Cong --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31503/#review76195 --- On March 10, 2015, 11:26 p.m., Cong Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31503/ --- (Updated March 10, 2015, 11:26 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Bugs: MESOS-2422 https://issues.apache.org/jira/browse/MESOS-2422 Repository: mesos Description --- Currently we don't support to read actions, so intead of using an action, we add the classid to the generic Filter class. Now it is much easier to read it for recovery. Diffs - src/linux/routing/filter/arp.hpp fa0ea6f93218fb3bee9f435f0268022d5edc5e11 src/linux/routing/filter/arp.cpp bf19264ccc83e8a8b8c2affe53779ac070472925 src/linux/routing/filter/filter.hpp d4ea099a2e35ffc0e6d560c5de9864dbbfe10038 src/linux/routing/filter/icmp.hpp 431bc19eb600b0568874dacfce123adef0f9ff0b src/linux/routing/filter/icmp.cpp 706b5d1871d9e0b79d1b869d27d0e87112b75975 src/linux/routing/filter/internal.hpp 8a6c0c0390c4ed119a11f0f808e0a244f7734c45 src/linux/routing/filter/ip.hpp b5406024fdbfd1510bcf619f04a7369763396e72 src/linux/routing/filter/ip.cpp de6407119c6726e0f32b07fd6ff61fa29262638b src/linux/routing/queueing/handle.hpp 2725d0794ca29ad5dc1b0148d0f68b90ce8b8369 src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba Diff: https://reviews.apache.org/r/31503/diff/ Testing --- Run the testcase. Thanks, Cong Wang
Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2530
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME/2530/changes Changes: [yujie.jay] Removed tailing white spaces in TestFramework.java. [yujie.jay] Added Java binding for the new acceptOffers API. [yujie.jay] Added Python binding for the acceptOffers API. -- [...truncated 82054 lines...] I0312 02:45:07.716696 29515 master.cpp:1592] Received registration request for framework 'default' at scheduler-3a2b5223-87f7-47c3-b981-e572bbbf97cb@67.195.81.187:56340 I0312 02:45:07.716773 29515 master.cpp:1453] Authorizing framework principal 'test-principal' to receive offers for role '*' I0312 02:45:07.717164 29518 master.cpp:1656] Registering framework 20150312-024507-3142697795-56340-29473- (default) at scheduler-3a2b5223-87f7-47c3-b981-e572bbbf97cb@67.195.81.187:56340 I0312 02:45:07.717530 29503 hierarchical.hpp:321] Added framework 20150312-024507-3142697795-56340-29473- I0312 02:45:07.717656 29506 sched.cpp:448] Framework registered with 20150312-024507-3142697795-56340-29473- I0312 02:45:07.717737 29506 sched.cpp:462] Scheduler::registered took 51119ns I0312 02:45:07.718034 29503 hierarchical.hpp:741] Performed allocation for 1 slaves in 472433ns I0312 02:45:07.718425 29517 master.cpp:3757] Sending 1 offers to framework 20150312-024507-3142697795-56340-29473- (default) at scheduler-3a2b5223-87f7-47c3-b981-e572bbbf97cb@67.195.81.187:56340 I0312 02:45:07.719357 29503 sched.cpp:611] Scheduler::resourceOffers took 631687ns I0312 02:45:07.720173 29509 master.cpp:2285] Processing ACCEPT call for offers: [ 20150312-024507-3142697795-56340-29473-O0 ] on slave 20150312-024507-3142697795-56340-29473-S0 at slave(236)@67.195.81.187:56340 (pomona.apache.org) for framework 20150312-024507-3142697795-56340-29473- (default) at scheduler-3a2b5223-87f7-47c3-b981-e572bbbf97cb@67.195.81.187:56340 I0312 02:45:07.720221 29509 master.cpp:2130] Authorizing framework principal 'test-principal' to launch task 0 as user 'jenkins' W0312 02:45:07.721595 29501 validation.cpp:326] Executor executor-1 for task 0 uses less CPUs (None) than the minimum required (0.01). Please update your executor, as this will be mandatory in future releases. W0312 02:45:07.721635 29501 validation.cpp:338] Executor executor-1 for task 0 uses less memory (None) than the minimum required (32MB). Please update your executor, as this will be mandatory in future releases. I0312 02:45:07.721938 29501 master.hpp:802] Adding task 0 with resources cpus(*):2; mem(*):1024 on slave 20150312-024507-3142697795-56340-29473-S0 (pomona.apache.org) I0312 02:45:07.722028 29501 master.cpp:2557] Launching task 0 of framework 20150312-024507-3142697795-56340-29473- (default) at scheduler-3a2b5223-87f7-47c3-b981-e572bbbf97cb@67.195.81.187:56340 with resources cpus(*):2; mem(*):1024 on slave 20150312-024507-3142697795-56340-29473-S0 at slave(236)@67.195.81.187:56340 (pomona.apache.org) I0312 02:45:07.722451 29514 slave.cpp:1109] Got assigned task 0 for framework 20150312-024507-3142697795-56340-29473- I0312 02:45:07.722523 29516 hierarchical.hpp:648] Recovered disk(*):1024; ports(*):[31000-32000] (total allocatable: disk(*):1024; ports(*):[31000-32000]) on slave 20150312-024507-3142697795-56340-29473-S0 from framework 20150312-024507-3142697795-56340-29473- I0312 02:45:07.722568 29516 hierarchical.hpp:684] Framework 20150312-024507-3142697795-56340-29473- filtered slave 20150312-024507-3142697795-56340-29473-S0 for 5secs I0312 02:45:07.722978 29514 slave.cpp:1219] Launching task 0 for framework 20150312-024507-3142697795-56340-29473- I0312 02:45:07.725734 29514 slave.cpp:4149] Launching executor executor-1 of framework 20150312-024507-3142697795-56340-29473- in work directory '/tmp/GarbageCollectorIntegrationTest_Unschedule_63CcbP/slaves/20150312-024507-3142697795-56340-29473-S0/frameworks/20150312-024507-3142697795-56340-29473-/executors/executor-1/runs/fdecabef-7fad-44c7-ab3a-16c7e355b3dd' I0312 02:45:07.728689 29514 exec.cpp:132] Version: 0.23.0 I0312 02:45:07.728942 29501 exec.cpp:182] Executor started at: executor(81)@67.195.81.187:56340 with pid 29473 I0312 02:45:07.729079 29514 slave.cpp:1365] Queuing task '0' for executor executor-1 of framework '20150312-024507-3142697795-56340-29473- I0312 02:45:07.729199 29514 slave.cpp:565] Successfully attached file '/tmp/GarbageCollectorIntegrationTest_Unschedule_63CcbP/slaves/20150312-024507-3142697795-56340-29473-S0/frameworks/20150312-024507-3142697795-56340-29473-/executors/executor-1/runs/fdecabef-7fad-44c7-ab3a-16c7e355b3dd' I0312 02:45:07.729336 29514 slave.cpp:3099] Monitoring executor 'executor-1' of framework '20150312-024507-3142697795-56340-29473-' in container 'fdecabef-7fad-44c7-ab3a-16c7e355b3dd' I0312 02:45:07.729509 29514 slave.cpp:2117] Got registration for executor 'executor-1' of framework 20150312-024507-3142697795-56340-29473- from
