Re: Review Request 26517: Symlink sandbox directories in docker containerizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/ --- (Updated Oct. 10, 2014, 6:35 a.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-1833 https://issues.apache.org/jira/browse/MESOS-1833 Repository: mesos-git Description (updated) --- Review: https://reviews.apache.org/r/26517 Diffs (updated) - src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 src/tests/docker_containerizer_tests.cpp 67d60a885d65edbcbbf702bce83a54d1a5c0411f Diff: https://reviews.apache.org/r/26517/diff/ Testing --- make check Thanks, Timothy Chen
Build failed in Jenkins: mesos-reviewbot #1937
See https://builds.apache.org/job/mesos-reviewbot/1937/
--
[...truncated 5307 lines...]
rm -f slave/containerizer/isolators/network/*.lo
rm -f slave/containerizer/mesos/*.o
rm -f slave/containerizer/mesos/*.lo
rm -f state/*.o
rm -f state/*.lo
rm -f tests/*.o
rm -rf authorizer/.libs authorizer/_libs
rm -rf common/.libs common/_libs
rm -rf containerizer/.libs containerizer/_libs
rm -rf docker/.libs docker/_libs
rm -rf examples/.libs examples/_libs
rm -rf exec/.libs exec/_libs
rm -rf files/.libs files/_libs
rm -rf java/jni/.libs java/jni/_libs
rm -rf jvm/.libs jvm/_libs
rm -rf jvm/org/apache/.libs jvm/org/apache/_libs
rm -rf linux/.libs linux/_libs
rm -rf linux/routing/.libs linux/routing/_libs
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 -f tests/common/*.o
rm -f usage/*.o
rm -f usage/*.lo
rm -f zookeeper/*.o
rm -f zookeeper/*.lo
rm -rf log/tool/.libs log/tool/_libs
rm -rf logging/.libs logging/_libs
rm -rf master/.libs master/_libs
rm -rf messages/.libs messages/_libs
rm -rf module/.libs module/_libs
rm -rf sasl/.libs sasl/_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 -rf slave/containerizer/isolators/network/.libs
slave/containerizer/isolators/network/_libs
rm -rf slave/containerizer/mesos/.libs slave/containerizer/mesos/_libs
rm -rf state/.libs state/_libs
rm -rf usage/.libs usage/_libs
rm -rf zookeeper/.libs zookeeper/_libs
rm -rf ./.deps authorizer/.deps cli/.deps common/.deps containerizer/.deps
docker/.deps examples/.deps exec/.deps files/.deps health-check/.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 messages/.deps module/.deps
sasl/.deps sched/.deps scheduler/.deps slave/.deps slave/containerizer/.deps
slave/containerizer/isolators/cgroups/.deps
slave/containerizer/isolators/network/.deps slave/containerizer/mesos/.deps
state/.deps tests/.deps tests/common/.deps usage/.deps zookeeper/.deps
rm -f Makefile
make[2]: Leaving directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.21.0/_build/src'
Making distclean in ec2
make[2]: Entering directory
`https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.21.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.21.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.21.0/_build'
if test -d mesos-0.21.0; then find mesos-0.21.0 -type d ! -perm -200 -exec
chmod u+w {} ';' rm -rf mesos-0.21.0 || { sleep 5 rm -rf
mesos-0.21.0; }; else :; fi
==
mesos-0.21.0 archives ready for distribution:
mesos-0.21.0.tar.gz
==
real18m3.846s
user113m44.342s
sys 7m12.019s
+ 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 docs Doxyfile ec2 frameworks include install-sh libtool
LICENSE ltmain.sh m4 Makefile Makefile.am Makefile.in mesos-0.21.0.tar.gz
mesos.pc mesos.pc.in missing mpi NOTICE README.md src support
+ git clean -fdx
Removing .libs/
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
Jenkins build is back to normal : mesos-reviewbot #1938
See https://builds.apache.org/job/mesos-reviewbot/1938/
Re: Review Request 26517: Symlink sandbox directories in docker containerizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/#review56115 --- Patch looks great! Reviews applied: [26517] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 6:35 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/ --- (Updated Oct. 10, 2014, 6:35 a.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-1833 https://issues.apache.org/jira/browse/MESOS-1833 Repository: mesos-git Description --- Review: https://reviews.apache.org/r/26517 Diffs - src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 src/tests/docker_containerizer_tests.cpp 67d60a885d65edbcbbf702bce83a54d1a5c0411f Diff: https://reviews.apache.org/r/26517/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56131 --- src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96461 Seems you should rephrase this towards path or adapt the protobuf within messages.proto to use file instead. - Till Toenshoff On Oct. 9, 2014, 11:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 11:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Oct. 9, 2014, 7:55 p.m., Timothy Chen wrote: src/tests/master_tests.cpp, line 265 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line265 Perhaps you should do EXPECT so you can cleanly shutdown in the end. If the response has not been parsed successfully, the test will fail right after. Same holds for assertions below. I mark this as dropped, but feel free to reopen if I vioalte the guidline or we have an agreed way to act here. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56041 --- On Oct. 9, 2014, 5:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 168 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line168 Any reason for these changes? Do you have any reference on if '' disambiguation is supported by our graced compilers? Here and below. To the best of my knowledge all supported compilers handle `` correctly and we agreed to make use of it in new patches. So I decided to clean-up while modifying the test. But can rollback it if you prefer the conservative way. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 165 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line165 Not yours, but can you add a comment describing the test? Sure. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 247 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line247 Mind expanding a bit on the expected state you are inspecting (and reason for the newly added code)? Expanded a comment above and one below. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, lines 253-255 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line253 Can you do this more reliably? For example setting an expectation for the message to be sent? If not - mind commenting on why? :-) My understanding is that this sequence guarantees all non-delayed messages in the mailbox are processed. I want to be sure that the `UnregisterFrameworkMessage` is processed by the master before I ask for the `state.json`. I've expanded a comment a bit, please let me know if there is a better way to achieve this. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56042 --- On Oct. 9, 2014, 5:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Oct. 10, 2014, 2:12 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till Toenshoff. Changes --- Change the variable name. Bugs: MESOS-1793 https://issues.apache.org/jira/browse/MESOS-1793 Repository: mesos-git Description --- Explicitly prohibit the use of namespace aliases. Give examples of preferable way of using auto. Diffs (updated) - docs/mesos-c++-style-guide.md 59a39df Diff: https://reviews.apache.org/r/25622/diff/ Testing --- Documentation change, no `make check` needed. Thanks, Alexander Rukletsov
Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.
On Oct. 9, 2014, 9:39 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, lines 98-101 https://reviews.apache.org/r/25622/diff/6/?file=716711#file716711line98 Why 'valueIt' here but 'i' below? How about 'i' in both cases? Ben, would you also update https://mesos.apache.org/documentation/latest/mesos-c++-style-guide/? And please check if the issue mentioned by MPark is gone away with this patch. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review56058 --- On Oct. 10, 2014, 2:12 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Oct. 10, 2014, 2:12 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till Toenshoff. Bugs: MESOS-1793 https://issues.apache.org/jira/browse/MESOS-1793 Repository: mesos-git Description --- Explicitly prohibit the use of namespace aliases. Give examples of preferable way of using auto. Diffs - docs/mesos-c++-style-guide.md 59a39df Diff: https://reviews.apache.org/r/25622/diff/ Testing --- Documentation change, no `make check` needed. Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 2:15 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Changes --- Fix the test (by using the new way to update the task status) and expand comments in the test. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs (updated) - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing (updated) --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review56142 --- Patch looks great! Reviews applied: [25622] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 2:12 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Oct. 10, 2014, 2:12 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till Toenshoff. Bugs: MESOS-1793 https://issues.apache.org/jira/browse/MESOS-1793 Repository: mesos-git Description --- Explicitly prohibit the use of namespace aliases. Give examples of preferable way of using auto. Diffs - docs/mesos-c++-style-guide.md 59a39df Diff: https://reviews.apache.org/r/25622/diff/ Testing --- Documentation change, no `make check` needed. Thanks, Alexander Rukletsov
Re: Review Request 26508: Added --module flag for Mesos master.
On Oct. 10, 2014, 9:17 a.m., Till Toenshoff wrote: src/master/flags.hpp, line 319 https://reviews.apache.org/r/26508/diff/4/?file=717127#file717127line319 Seems you should rephrase this towards path or adapt the protobuf within messages.proto to use file instead. There is already a review out to update the protobuf :-) https://reviews.apache.org/r/26529/. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56131 --- On Oct. 9, 2014, 7:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 7:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26508: Added --module flag for Mesos master.
On Oct. 10, 2014, 1:17 p.m., Till Toenshoff wrote: src/master/flags.hpp, line 319 https://reviews.apache.org/r/26508/diff/4/?file=717127#file717127line319 Seems you should rephrase this towards path or adapt the protobuf within messages.proto to use file instead. Kapil Arya wrote: There is already a review out to update the protobuf :-) https://reviews.apache.org/r/26529/. Hah, indeed - I should have checked the dependencies first. Thanks for clearifying! - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56131 --- On Oct. 9, 2014, 11:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 11:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 11:24 a.m.) Review request for mesos and Niklas Nielsen. Changes --- filename-file; replaced macros with const char*. Summary (updated) - Modules should accept relative path or file name for library name. Repository: mesos-git Description (updated) --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs (updated) - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 --- src/module/manager.cpp https://reviews.apache.org/r/26529/#comment96471 Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) src/tests/module_tests.cpp https://reviews.apache.org/r/26529/#comment96470 Let's try to avoid macro constants - can we do static const char* or define a helper that does this for you? src/tests/module_tests.cpp https://reviews.apache.org/r/26529/#comment96472 const strings if you don't intent to change them How about hoisting the 'LD_LIBRARY_PATH' string and define it once as a constant? - Niklas Nielsen On Oct. 10, 2014, 8:24 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 8:24 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
On Oct. 10, 2014, 11:31 a.m., Niklas Nielsen wrote: src/module/manager.cpp, lines 191-193 https://reviews.apache.org/r/26529/diff/2/?file=717123#file717123line191 Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) Should I create a separate review request for the empty test(s)? - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 --- On Oct. 10, 2014, 11:24 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 11:24 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 26529: Modules should accept relative path or file name for library name.
On Oct. 10, 2014, 8:31 a.m., Niklas Nielsen wrote: src/module/manager.cpp, lines 191-193 https://reviews.apache.org/r/26529/diff/2/?file=717123#file717123line191 Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) Kapil Arya wrote: Should I create a separate review request for the empty test(s)? One or the other. The commit didn't include any details on the bug you fixed when renaming 'path' - I will leave that up to you. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 --- On Oct. 10, 2014, 8:24 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 8:24 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56155 --- Patch looks great! Reviews applied: [25250] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 2:15 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 2:15 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56157 --- src/master/flags.hpp https://reviews.apache.org/r/26508/#comment96476 Can we include the JSON syntax here instead of '...'? - Niklas Nielsen On Oct. 9, 2014, 4:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 4:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25525/#review56158
---
src/common/attributes.cpp
https://reviews.apache.org/r/25525/#comment96481
{ on next line
src/common/attributes.cpp
https://reviews.apache.org/r/25525/#comment96482
{ on next line
src/master/hierarchical_allocator_process.hpp
https://reviews.apache.org/r/25525/#comment96478
{} on new line
src/master/hierarchical_allocator_process.hpp
https://reviews.apache.org/r/25525/#comment96477
Add '.' to comment
- Cody Maloney
On Oct. 10, 2014, 2:29 a.m., Cody Maloney wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25525/
---
(Updated Oct. 10, 2014, 2:29 a.m.)
Review request for mesos, Adam B and Vinod Kone.
Bugs: MESOS-1739
https://issues.apache.org/jira/browse/MESOS-1739
Repository: mesos-git
Description
---
Allows attributes and resources to be set to a superset of what they were
previously on a slave restart.
Incorporates all comments from:
https://issues.apache.org/jira/browse/MESOS-1739
and the former review request:
https://reviews.apache.org/r/25111/
Diffs
-
src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4
src/common/attributes.hpp 0a043d5b5dca804c6dd215cabd2704f24df71a33
src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21
src/common/slaveinfo_utils.hpp PRE-CREATION
src/common/slaveinfo_utils.cpp PRE-CREATION
src/master/allocator.hpp 02d20d0cc802805bc702891306aa42894531b223
src/master/hierarchical_allocator_process.hpp
31dfb2cc86e2e74da43f05fbada10976ce65f3e4
src/master/master.hpp 14f1d0fd240c9cd0718d0238e1fbb9c733190205
src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d
src/slave/slave.cpp cb3759993f863590cb1545c73072feb0331aa6c9
src/tests/attributes_tests.cpp 240a8cac18ac12178cf73e8eeb88bd50e3fcc03b
src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104
src/tests/slave_tests.cpp 69be28f6e82b99e23424bd2be8294f715d8040d4
Diff: https://reviews.apache.org/r/25525/diff/
Testing
---
make check on localhost
Thanks,
Cody Maloney
Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56165 --- src/master/registrar.cpp https://reviews.apache.org/r/26535/#comment96485 could you use the metrics timer instead (and still log the result) so that it is exposed on the metrics endpoint? - Dominic Hamon On Oct. 9, 2014, 6:51 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- (Updated Oct. 9, 2014, 6:51 p.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- We currently lack logging visibility into the batch sizes in the registrar. While we have timing metrics, we also lack the ability to easily read timings for particular operations. Diffs - src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552 Diff: https://reviews.apache.org/r/26535/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 26533: Memory cleanup: libprocess finalize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/#review56166 --- 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26533/#comment96486 should these be std::unique_ptr instead? (i'm currently adding support for this to the libprocess configure check). - Dominic Hamon On Oct. 9, 2014, 6:02 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/ --- (Updated Oct. 9, 2014, 6:02 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Introduce a finalize() function to match the initialize() function in process.hpp. Use this at the unit test runner for libprocess to start getting more valuable valgrind feedback. - Added cleanup of remaining Processes in ~ProcessManager() as a start. Diffs - 3rdparty/libprocess/include/process/process.hpp 270ca28 3rdparty/libprocess/src/process.cpp d30ed63 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e Diff: https://reviews.apache.org/r/26533/diff/ Testing --- make check support/mesos-style valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost blocks) Thanks, Joris Van Remoortere
Re: Review Request 26533: Memory cleanup: libprocess finalize
On Oct. 10, 2014, 5:04 p.m., Dominic Hamon wrote: 3rdparty/libprocess/src/process.cpp, line 1453 https://reviews.apache.org/r/26533/diff/1/?file=717191#file717191line1453 should these be std::unique_ptr instead? (i'm currently adding support for this to the libprocess configure check). Indeed. I have some follow up patches that will try to tackle the actual clean shutdown. This initial patch it to try and introduce a place to close the loop. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/#review56166 --- On Oct. 10, 2014, 1:02 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/ --- (Updated Oct. 10, 2014, 1:02 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Introduce a finalize() function to match the initialize() function in process.hpp. Use this at the unit test runner for libprocess to start getting more valuable valgrind feedback. - Added cleanup of remaining Processes in ~ProcessManager() as a start. Diffs - 3rdparty/libprocess/include/process/process.hpp 270ca28 3rdparty/libprocess/src/process.cpp d30ed63 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e Diff: https://reviews.apache.org/r/26533/diff/ Testing --- make check support/mesos-style valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost blocks) Thanks, Joris Van Remoortere
Re: Jekin build steps
I commented on the ticket. It's not a problem with Mesos. On Wed, Oct 8, 2014 at 12:11 AM, Klaus Ma klaus...@outlook.com wrote: Hi all, I'm working on MESOS-1416 (mesos-0.19.0 build directory is read-only), so i'd like to know the steps to reproduce it. Can anyone help me? Regards,Da Ma (马达), PMP® | CEL3 Team LeadPlatform Symphony MapReduce Development Support, STG, IBM GCG+86-10-8245 4084 | mad...@cn.ibm.com | http://www.cguru.net
Re: Review Request 26533: Memory cleanup: libprocess finalize
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26533/#review56177
---
Ship it!
LGTM.
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/26533/#comment96500
We usually prefer an explict representation:
```
if (ptr != nullptr) {
}
```
Also, probably we should create a ticket to convert all NULL to nullptr in
our code base:)
- Jie Yu
On Oct. 10, 2014, 1:02 a.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26533/
---
(Updated Oct. 10, 2014, 1:02 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
---
Introduce a finalize() function to match the initialize() function in
process.hpp. Use this at the unit test runner for libprocess to start getting
more valuable valgrind feedback.
- Added cleanup of remaining Processes in ~ProcessManager() as a start.
Diffs
-
3rdparty/libprocess/include/process/process.hpp 270ca28
3rdparty/libprocess/src/process.cpp d30ed63
3rdparty/libprocess/src/tests/main.cpp 0a3ba4e
Diff: https://reviews.apache.org/r/26533/diff/
Testing
---
make check
support/mesos-style
valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost
blocks)
Thanks,
Joris Van Remoortere
Re: Review Request 26525: Moved executor checkpointing code from the Executor constructor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/#review56179 --- src/slave/slave.cpp https://reviews.apache.org/r/26525/#comment96501 I think it is weird to call a checkpoint*() function that is a no-op if it is not checkpointing. It is my bad, because I realize I did the same mistake with checkpointTask(). Can you fix both checkpointExecutor() and checkpointTask() to only be called when we are checkpointing? src/slave/slave.cpp https://reviews.apache.org/r/26525/#comment96502 This should be a CHECK once you fix the callers. src/slave/slave.cpp https://reviews.apache.org/r/26525/#comment96504 Change this to VLOG(1). src/slave/slave.cpp https://reviews.apache.org/r/26525/#comment96503 ditto. this should be a CHECK. - Vinod Kone On Oct. 9, 2014, 9:39 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/ --- (Updated Oct. 9, 2014, 9:39 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- There are two places where 'new Executor' is called: 1) launchExecutor 2) recoverExecutor For 2), we don't need checkpointing. Therefore, putting checkpointing code in Executor constructor and use state != RECOVERING to disginguish is not explicit and confusing. Diffs - src/slave/slave.hpp 28697102047b972ecb3b6b627ee089b430549fc0 src/slave/slave.cpp e56dcbd80114730949a0d4b553470802a4d38281 Diff: https://reviews.apache.org/r/26525/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 24537: Updated metrics::Timer::stop to return elapsed time.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24537/#review56180 --- Hey Ben, I've committed this for you as I would like to leverage this in r/26535. - Ben Mahler On Sept. 29, 2014, 12:24 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24537/ --- (Updated Sept. 29, 2014, 12:24 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- See summary. Diffs - 3rdparty/libprocess/include/process/metrics/timer.hpp dfd7dd951f0997689dc08fed02a6e621c8d4683f Diff: https://reviews.apache.org/r/24537/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56183 --- Ship it! Ship It! - Niklas Nielsen On Oct. 9, 2014, 4:19 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 4:19 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.
On Oct. 7, 2014, 5:32 p.m., Jie Yu wrote: Can you use unique_ptr directly now? If yes, I would suggest using unique_ptr directly in libprocess. Here are the reasons: 1) We use lots of shared_ptr in libprocess. Mixing Owned with shared_ptr seems to be confusing. 2) What if the implementation of Owned/Shared depends on dispatch, there will be cyclic dependencies (though it's not a problem now). 3) I think directly using std::shared_ptr and std::unique_ptr is fine in libprocess (we even use pthread in libprocess:)) What do you think? Michael Park wrote: +1, I'd like to see movement toward `std::unique_ptr` and `std::shared_ptr` as well. Both are available in `gcc-4.4` and up. Dominic Hamon wrote: I discussed this with vinod and he prefered a migration approach: 1. migrate to Owned pointer to clarify ownership 2. replace Owned with unique_ptr I was originally going to move straight to unique_ptr (yes we can use it now in g++-4.4, and std::move which is important for unique_ptr in APIs) but that was his suggestion. I'm open to the alternative but we'll need to hash it out with him. Jie Yu wrote: Talked to Vinod. The consensus is: for libprocess primitives (essential for the runtime like dispatch, delay, Future, ProcessBase, SocketManager, etc.), we should use std::unique_ptr and std::shared_ptr directly. For Mesos code, try to use Owned and Shared as much as possible. for c++11 this is trivial. for earlier compilers we run into some issues trying to bind to move-only types. I didn't see the issues with process::Owned because it is actually a shared_ptr (hence copyable) under the hood. I'm not seeing a clean way around this at this point. Perhaps we can have a C++11 'dispatch' high-level method that uses the unique_ptr and one that uses Owned for older compilers? I'm nervous about taking this difference up the stack though. - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/#review55748 --- On Oct. 7, 2014, 5:15 p.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/ --- (Updated Oct. 7, 2014, 5:15 p.m.) Review request for mesos and Jie Yu. Repository: mesos-git Description --- replace shared_ptr with Owned where ownership is being passed. Diffs - 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4 3rdparty/libprocess/include/process/defer.hpp ebe6f2db47cab2a3306288d8ebabb720e7cd8976 3rdparty/libprocess/include/process/delay.hpp 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227 3rdparty/libprocess/include/process/event.hpp bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 3rdparty/libprocess/src/process.cpp d30ed636ee24d0fe6e62f69a921307bde1f32765 Diff: https://reviews.apache.org/r/26289/diff/ Testing --- make g++-4.4 and clang++-3.5 make check clang++-3.5 Thanks, Dominic Hamon
Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review56016 --- src/slave/slave.cpp https://reviews.apache.org/r/23912/#comment96370 why the change here? is it possible for pending[executorId] to be empty() if you didn't erase the task just above? i would revert this. src/tests/slave_tests.cpp https://reviews.apache.org/r/23912/#comment96505 why this change? src/tests/slave_tests.cpp https://reviews.apache.org/r/23912/#comment96506 How can removeFramework() be called twice!!! Wouldn't that throw a CHECK failure? src/slave/slave.cpp https://reviews.apache.org/r/23912/#comment96511 This seems out of place to me. See my comments below. src/slave/slave.cpp https://reviews.apache.org/r/23912/#comment96510 It is weird to me that you remove the task here but (potentially) remove the executor up in _runTask(). It's not obvious to me why you made that choice. If there is a good reason, please add a comment here. - Vinod Kone On Oct. 9, 2014, 2:10 p.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 9, 2014, 2:10 p.m.) Review request for mesos. Bugs: MESOS-947 https://issues.apache.org/jira/browse/MESOS-947 Repository: mesos-git Description --- Fixes MESOS-947 Slave should properly handle a killTask() that arrives between runTask() and _runTask(). Slave::killTask() did not check for task in question combination to be pending (i.e. Slave::runTask had happened, but Slave::_runTask had not yet) and then erroneously assumed that Slave::runTask() had not been executed. The task was then marked LOST instead of KILLED. But Slave::runTask had already scheduled Slave::_runTask to follow. Now the entry for being pending is removed, and the task is marked KILLED, and _runTask gets informed about this. It checks whether the task in question is currently pending and if it is not, then it infers that the task has been killed and does not erroneously try to complete launching it. Diffs - src/slave/slave.hpp 76d505c698774204b2536b66ea8a83a9a2a5e2c1 src/slave/slave.cpp cb3759993f863590cb1545c73072feb0331aa6c9 src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 src/tests/mesos.cpp 3dcb2acd5ad4ab5e3a7b4fe524ee077558112773 src/tests/slave_tests.cpp 69be28f6e82b99e23424bd2be8294f715d8040d4 Diff: https://reviews.apache.org/r/23912/diff/ Testing --- Wrote a unit test that reliably created the situation described in the ticket. Observed that TASK_LOST and the listed log output occurred. This pointed directly to the lines in killTask() where the problem is rooted. Ran the test after fixing, it succeeded. Checked the log. It looks like a clean kill now :-) Thanks, Bernd Mathiske
Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- (Updated Oct. 10, 2014, 6:25 p.m.) Review request for mesos and Vinod Kone. Changes --- Updated to use Timer::stop() result from https://reviews.apache.org/r/24537/ per Dominic's comment. Hopefully this improves the accuracy of the timings as well (timer is now started before calls on 'state' occur). Repository: mesos-git Description --- We currently lack logging visibility into the batch sizes in the registrar. While we have timing metrics, we also lack the ability to easily read timings for particular operations. Diffs (updated) - src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552 Diff: https://reviews.apache.org/r/26535/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56188 --- src/master/registrar.cpp https://reviews.apache.org/r/26535/#comment96512 we should probably stop the timer whether the recovery succeeded or failed. maybe we need a 'cancel' method on the timer? - Dominic Hamon On Oct. 10, 2014, 11:25 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- (Updated Oct. 10, 2014, 11:25 a.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- We currently lack logging visibility into the batch sizes in the registrar. While we have timing metrics, we also lack the ability to easily read timings for particular operations. Diffs - src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552 Diff: https://reviews.apache.org/r/26535/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.
On Oct. 10, 2014, 6:27 p.m., Dominic Hamon wrote: src/master/registrar.cpp, line 341 https://reviews.apache.org/r/26535/diff/2/?file=717559#file717559line341 we should probably stop the timer whether the recovery succeeded or failed. maybe we need a 'cancel' method on the timer? I thought about this, is there any value in including the timing of failures? At least for the registrar, the point is moot since we'll fail the master. More generally, since 'start()' will reset the timer, it seems safe to exclude certain timings (e.g. failures) by not calling stop() for these cases. Thoughts? - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56188 --- On Oct. 10, 2014, 6:25 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- (Updated Oct. 10, 2014, 6:25 p.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- We currently lack logging visibility into the batch sizes in the registrar. While we have timing metrics, we also lack the ability to easily read timings for particular operations. Diffs - src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552 Diff: https://reviews.apache.org/r/26535/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 26533: Memory cleanup: libprocess finalize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/#review56186 --- Ship it! 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26533/#comment96508 We've used s/ptr/process throughout the rest of the code, please be consistent on variable naming! 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26533/#comment96507 Was I the only one that had to think about what you were doing here and why you had to do it the way you're doing it? ;-) Please add comments. 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26533/#comment96509 Why not use '!empty()'? Or if you use .size(), compare it with a number please. 3rdparty/libprocess/src/tests/main.cpp https://reviews.apache.org/r/26533/#comment96515 We've used s/ret/result/ in the code base, let's stay consistent please. - Benjamin Hindman On Oct. 10, 2014, 1:02 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/ --- (Updated Oct. 10, 2014, 1:02 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Introduce a finalize() function to match the initialize() function in process.hpp. Use this at the unit test runner for libprocess to start getting more valuable valgrind feedback. - Added cleanup of remaining Processes in ~ProcessManager() as a start. Diffs - 3rdparty/libprocess/include/process/process.hpp 270ca28 3rdparty/libprocess/src/process.cpp d30ed63 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e Diff: https://reviews.apache.org/r/26533/diff/ Testing --- make check support/mesos-style valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost blocks) Thanks, Joris Van Remoortere
Re: Review Request 26470: Removed unused resources and duplicated attributes from Slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26470/#review56193 --- Ship it! Feel free to add tests in a separate review, this looks right but would be great to at least verify manually before you commit this. - Ben Mahler On Oct. 9, 2014, 12:29 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26470/ --- (Updated Oct. 9, 2014, 12:29 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- See summary. Right now, the 'resources' obtained from localhost:5051/state.json is not correct. Diffs - src/slave/http.cpp ec7c6b992f72246db0044734785c4d851a27734a src/slave/slave.hpp 28697102047b972ecb3b6b627ee089b430549fc0 src/slave/slave.cpp 809b008b1502b80cce4d8b4be0a233117c92ed56 Diff: https://reviews.apache.org/r/26470/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 6:49 p.m.) Review request for mesos, Adam B and Vinod Kone. Changes --- Fix style issues Bugs: MESOS-1739 https://issues.apache.org/jira/browse/MESOS-1739 Repository: mesos-git Description --- Allows attributes and resources to be set to a superset of what they were previously on a slave restart. Incorporates all comments from: https://issues.apache.org/jira/browse/MESOS-1739 and the former review request: https://reviews.apache.org/r/25111/ Diffs (updated) - src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 src/common/attributes.hpp 0a043d5b5dca804c6dd215cabd2704f24df71a33 src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21 src/common/slaveinfo_utils.hpp PRE-CREATION src/common/slaveinfo_utils.cpp PRE-CREATION src/master/allocator.hpp 02d20d0cc802805bc702891306aa42894531b223 src/master/hierarchical_allocator_process.hpp 31dfb2cc86e2e74da43f05fbada10976ce65f3e4 src/master/master.hpp 14f1d0fd240c9cd0718d0238e1fbb9c733190205 src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d src/slave/slave.cpp cb3759993f863590cb1545c73072feb0331aa6c9 src/tests/attributes_tests.cpp 240a8cac18ac12178cf73e8eeb88bd50e3fcc03b src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 src/tests/slave_tests.cpp 69be28f6e82b99e23424bd2be8294f715d8040d4 Diff: https://reviews.apache.org/r/25525/diff/ Testing --- make check on localhost Thanks, Cody Maloney
Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.
On Oct. 8, 2014, 12:32 a.m., Jie Yu wrote: Can you use unique_ptr directly now? If yes, I would suggest using unique_ptr directly in libprocess. Here are the reasons: 1) We use lots of shared_ptr in libprocess. Mixing Owned with shared_ptr seems to be confusing. 2) What if the implementation of Owned/Shared depends on dispatch, there will be cyclic dependencies (though it's not a problem now). 3) I think directly using std::shared_ptr and std::unique_ptr is fine in libprocess (we even use pthread in libprocess:)) What do you think? Michael Park wrote: +1, I'd like to see movement toward `std::unique_ptr` and `std::shared_ptr` as well. Both are available in `gcc-4.4` and up. Dominic Hamon wrote: I discussed this with vinod and he prefered a migration approach: 1. migrate to Owned pointer to clarify ownership 2. replace Owned with unique_ptr I was originally going to move straight to unique_ptr (yes we can use it now in g++-4.4, and std::move which is important for unique_ptr in APIs) but that was his suggestion. I'm open to the alternative but we'll need to hash it out with him. Jie Yu wrote: Talked to Vinod. The consensus is: for libprocess primitives (essential for the runtime like dispatch, delay, Future, ProcessBase, SocketManager, etc.), we should use std::unique_ptr and std::shared_ptr directly. For Mesos code, try to use Owned and Shared as much as possible. Dominic Hamon wrote: for c++11 this is trivial. for earlier compilers we run into some issues trying to bind to move-only types. I didn't see the issues with process::Owned because it is actually a shared_ptr (hence copyable) under the hood. I'm not seeing a clean way around this at this point. Perhaps we can have a C++11 'dispatch' high-level method that uses the unique_ptr and one that uses Owned for older compilers? I'm nervous about taking this difference up the stack though. Can you paste the gcc version and the error message? I am curious to know. I am fine with use Owned for now with a TODO somewhere sayting that we should use unique_ptr directly for libprocess primitives. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/#review55748 --- On Oct. 8, 2014, 12:15 a.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/ --- (Updated Oct. 8, 2014, 12:15 a.m.) Review request for mesos and Jie Yu. Repository: mesos-git Description --- replace shared_ptr with Owned where ownership is being passed. Diffs - 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4 3rdparty/libprocess/include/process/defer.hpp ebe6f2db47cab2a3306288d8ebabb720e7cd8976 3rdparty/libprocess/include/process/delay.hpp 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227 3rdparty/libprocess/include/process/event.hpp bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 3rdparty/libprocess/src/process.cpp d30ed636ee24d0fe6e62f69a921307bde1f32765 Diff: https://reviews.apache.org/r/26289/diff/ Testing --- make g++-4.4 and clang++-3.5 make check clang++-3.5 Thanks, Dominic Hamon
Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56200 --- Patch looks great! Reviews applied: [26535] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 6:25 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- (Updated Oct. 10, 2014, 6:25 p.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- We currently lack logging visibility into the batch sizes in the registrar. While we have timing metrics, we also lack the ability to easily read timings for particular operations. Diffs - src/master/registrar.cpp ccc2fd23b1a1d56ab327cc73f6fef35d4bf6a552 Diff: https://reviews.apache.org/r/26535/diff/ Testing --- make check Thanks, Ben Mahler
Review Request 26571: Fixed ZooKeeper 3.4.5 OSX Yosemite build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/ --- Review request for mesos. Bugs: MESOS-1797 https://issues.apache.org/jira/browse/MESOS-1797 Repository: mesos-git Description --- Fixes build issue caused by htonll introduced by OSX 10.10 (Yosemite). This patch has been submitted upstream as well: https://issues.apache.org/jira/browse/ZOOKEEPER-2049 This patch shall not be committed until upstream has accepted it - I will update this accordingly. Diffs - 3rdparty/zookeeper-3.4.5.patch PRE-CREATION Diff: https://reviews.apache.org/r/26571/diff/ Testing --- make check (OSX 10.10 BETA5) Thanks, Till Toenshoff
Re: Review Request 26470: Removed unused resources and duplicated attributes from Slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26470/ --- (Updated Oct. 10, 2014, 7:41 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added a test. Repository: mesos-git Description --- See summary. Right now, the 'resources' obtained from localhost:5051/state.json is not correct. Diffs (updated) - src/slave/http.cpp ec7c6b992f72246db0044734785c4d851a27734a src/slave/slave.hpp 76d505c698774204b2536b66ea8a83a9a2a5e2c1 src/slave/slave.cpp cb3759993f863590cb1545c73072feb0331aa6c9 src/tests/slave_tests.cpp 69be28f6e82b99e23424bd2be8294f715d8040d4 Diff: https://reviews.apache.org/r/26470/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.
On Oct. 7, 2014, 5:32 p.m., Jie Yu wrote: Can you use unique_ptr directly now? If yes, I would suggest using unique_ptr directly in libprocess. Here are the reasons: 1) We use lots of shared_ptr in libprocess. Mixing Owned with shared_ptr seems to be confusing. 2) What if the implementation of Owned/Shared depends on dispatch, there will be cyclic dependencies (though it's not a problem now). 3) I think directly using std::shared_ptr and std::unique_ptr is fine in libprocess (we even use pthread in libprocess:)) What do you think? Michael Park wrote: +1, I'd like to see movement toward `std::unique_ptr` and `std::shared_ptr` as well. Both are available in `gcc-4.4` and up. Dominic Hamon wrote: I discussed this with vinod and he prefered a migration approach: 1. migrate to Owned pointer to clarify ownership 2. replace Owned with unique_ptr I was originally going to move straight to unique_ptr (yes we can use it now in g++-4.4, and std::move which is important for unique_ptr in APIs) but that was his suggestion. I'm open to the alternative but we'll need to hash it out with him. Jie Yu wrote: Talked to Vinod. The consensus is: for libprocess primitives (essential for the runtime like dispatch, delay, Future, ProcessBase, SocketManager, etc.), we should use std::unique_ptr and std::shared_ptr directly. For Mesos code, try to use Owned and Shared as much as possible. Dominic Hamon wrote: for c++11 this is trivial. for earlier compilers we run into some issues trying to bind to move-only types. I didn't see the issues with process::Owned because it is actually a shared_ptr (hence copyable) under the hood. I'm not seeing a clean way around this at this point. Perhaps we can have a C++11 'dispatch' high-level method that uses the unique_ptr and one that uses Owned for older compilers? I'm nervous about taking this difference up the stack though. Jie Yu wrote: Can you paste the gcc version and the error message? I am curious to know. I am fine with use Owned for now with a TODO somewhere sayting that we should use unique_ptr directly for libprocess primitives. it's anything that doesn't use the c++11 implementations of dispatch/defer. The error message is from this: http://stackoverflow.com/questions/9955714/does-stdbind-work-with-move-only-types-in-general-and-stdunique-ptr-in-part. Essentially, binding a move-only arg creates a move-only functor which causes issues further up the stack. My workaround (which compiles for g++-4.4, g++-4.8, and i'm testing 4.6 and clang) is to have two top-level dispatch methods: one takes Owned and releases it into a unique_ptr, then forwards to the other that takes a unique_ptr directly. I think there's another change in flight that replaces much of this code with variadic templated versions so we can revisit once it's all simplified. - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/#review55748 --- On Oct. 7, 2014, 5:15 p.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/ --- (Updated Oct. 7, 2014, 5:15 p.m.) Review request for mesos and Jie Yu. Repository: mesos-git Description --- replace shared_ptr with Owned where ownership is being passed. Diffs - 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4 3rdparty/libprocess/include/process/defer.hpp ebe6f2db47cab2a3306288d8ebabb720e7cd8976 3rdparty/libprocess/include/process/delay.hpp 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227 3rdparty/libprocess/include/process/event.hpp bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 3rdparty/libprocess/src/process.cpp d30ed636ee24d0fe6e62f69a921307bde1f32765 Diff: https://reviews.apache.org/r/26289/diff/ Testing --- make g++-4.4 and clang++-3.5 make check clang++-3.5 Thanks, Dominic Hamon
Re: Review Request 26533: Memory cleanup: libprocess finalize
On Oct. 10, 2014, 6:02 p.m., Jie Yu wrote:
3rdparty/libprocess/src/process.cpp, line 2449
https://reviews.apache.org/r/26533/diff/1/?file=717191#file717191line2449
We usually prefer an explict representation:
```
if (ptr != nullptr) {
}
```
Also, probably we should create a ticket to convert all NULL to nullptr
in our code base:)
Unfortunately, `nullptr` is not available in gcc-4.4 :(
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26533/#review56177
---
On Oct. 10, 2014, 1:02 a.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26533/
---
(Updated Oct. 10, 2014, 1:02 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
---
Introduce a finalize() function to match the initialize() function in
process.hpp. Use this at the unit test runner for libprocess to start getting
more valuable valgrind feedback.
- Added cleanup of remaining Processes in ~ProcessManager() as a start.
Diffs
-
3rdparty/libprocess/include/process/process.hpp 270ca28
3rdparty/libprocess/src/process.cpp d30ed63
3rdparty/libprocess/src/tests/main.cpp 0a3ba4e
Diff: https://reviews.apache.org/r/26533/diff/
Testing
---
make check
support/mesos-style
valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost
blocks)
Thanks,
Joris Van Remoortere
Re: Review Request 26470: Removed unused resources and duplicated attributes from Slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26470/#review56212 --- Ship it! src/tests/slave_tests.cpp https://reviews.apache.org/r/26470/#comment96535 s/get()/get()/ ? - Ben Mahler On Oct. 10, 2014, 7:41 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26470/ --- (Updated Oct. 10, 2014, 7:41 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- See summary. Right now, the 'resources' obtained from localhost:5051/state.json is not correct. Diffs - src/slave/http.cpp ec7c6b992f72246db0044734785c4d851a27734a src/slave/slave.hpp 76d505c698774204b2536b66ea8a83a9a2a5e2c1 src/slave/slave.cpp cb3759993f863590cb1545c73072feb0331aa6c9 src/tests/slave_tests.cpp 69be28f6e82b99e23424bd2be8294f715d8040d4 Diff: https://reviews.apache.org/r/26470/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 26274: Remove /proc and /sys remounts from port_mapping isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26274/ --- (Updated Oct. 10, 2014, 1:48 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Changes --- Corrected a comment. Bugs: MESOS-1853 https://issues.apache.org/jira/browse/MESOS-1853 Repository: mesos-git Description --- Remove /proc and /sys remounts from port_mapping isolator. See ticket for details. Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.cpp 8ddfb1817d78c45ce2f6daebbeb13d380998ee4d src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b Diff: https://reviews.apache.org/r/26274/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 26274: Remove /proc and /sys remounts from port_mapping isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26274/#review56215 --- Ship it! Please make sure to test it before committing this. - Jie Yu On Oct. 10, 2014, 8:48 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26274/ --- (Updated Oct. 10, 2014, 8:48 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Bugs: MESOS-1853 https://issues.apache.org/jira/browse/MESOS-1853 Repository: mesos-git Description --- Remove /proc and /sys remounts from port_mapping isolator. See ticket for details. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 8ddfb1817d78c45ce2f6daebbeb13d380998ee4d src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b Diff: https://reviews.apache.org/r/26274/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 26525: Moved executor checkpointing code from the Executor constructor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/#review56219 --- Ship it! Ship It! - Vinod Kone On Oct. 10, 2014, 8:40 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/ --- (Updated Oct. 10, 2014, 8:40 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git Description --- There are two places where 'new Executor' is called: 1) launchExecutor 2) recoverExecutor For 2), we don't need checkpointing. Therefore, putting checkpointing code in Executor constructor and use state != RECOVERING to disginguish is not explicit and confusing. Diffs - src/slave/slave.hpp 76d505c src/slave/slave.cpp cb37599 Diff: https://reviews.apache.org/r/26525/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 26571: Fixed ZooKeeper 3.4.5 OSX Yosemite build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/#review56221 --- Patch looks great! Reviews applied: [26571] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 7:19 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/ --- (Updated Oct. 10, 2014, 7:19 p.m.) Review request for mesos. Bugs: MESOS-1797 https://issues.apache.org/jira/browse/MESOS-1797 Repository: mesos-git Description --- Fixes build issue caused by htonll introduced by OSX 10.10 (Yosemite). This patch has been submitted upstream as well: https://issues.apache.org/jira/browse/ZOOKEEPER-2049 This patch shall not be committed until upstream has accepted it - I will update this accordingly. Diffs - 3rdparty/zookeeper-3.4.5.patch PRE-CREATION Diff: https://reviews.apache.org/r/26571/diff/ Testing --- make check (OSX 10.10 BETA5) Thanks, Till Toenshoff
Re: Review Request 26396: Exposed p90/95/99 for tcp rtt container stats.
On Oct. 7, 2014, 1:02 a.m., Dominic Hamon wrote: src/slave/containerizer/isolators/network/port_mapping.cpp, line 587 https://reviews.apache.org/r/26396/diff/1/?file=714484#file714484line587 for the record, this isn't completely accurate. see https://github.com/apache/mesos/blob/master/3rdparty/libprocess/include/process/statistics.hpp#L76 for how to correctly interpolate for statistics. or use the TimeSeries to track RTTs and use the statistics code directly instead of reinventing the wheel. Use TimeSeries is not straightfoward as it requires timestamps. I added a TODO here. We might wanna move Statistics to stout and get rid of the dependency to TimeSeries. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26396/#review55624 --- On Oct. 7, 2014, 12:38 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26396/ --- (Updated Oct. 7, 2014, 12:38 a.m.) Review request for mesos, Chi Zhang and Ian Downes. Bugs: MESOS-1808 https://issues.apache.org/jira/browse/MESOS-1808 Repository: mesos-git Description --- See summary. I did a few cleanup too. Diffs - include/mesos/mesos.proto f536017ce15fad7917818a30e91e6ff45602cf16 src/slave/containerizer/isolators/network/port_mapping.hpp 5bc7edd8a3d9c9095226f6e21df3e7a3d180ab48 src/slave/containerizer/isolators/network/port_mapping.cpp 88ae509f7962ae90a76823f91083ab27bef2be1e src/tests/port_mapping_tests.cpp 53ba8f30b12a8b89c22c357a5930fe34ba2da445 Diff: https://reviews.apache.org/r/26396/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 26529: Modules should accept relative path or file name for library name.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26529/
---
(Updated Oct. 10, 2014, 5:32 p.m.)
Review request for mesos and Niklas Nielsen.
Changes
---
Marked {old,new}LdPreload as const.
Repository: mesos-git
Description
---
Rename path in Modules protobuf to file since a path always referes to an
absolute or a relative path, whereas file may refer to a path or a file name.
If file contains a slash (/), it is considered an absolute or relative
path, otherwise if is considered a file name. For file name, dlopen()
automatically does a standard library path search to find the absolute path.
Also, return a better error message if an empty module name is provided.
Diffs (updated)
-
include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce
src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1
src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc
src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc
Diff: https://reviews.apache.org/r/26529/diff/
Testing
---
Added tests for LD_LIBRARY_PATH and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26533: Memory cleanup: libprocess finalize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/ --- (Updated Oct. 10, 2014, 9:39 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Changes --- fix style issues. Repository: mesos-git Description --- Introduce a finalize() function to match the initialize() function in process.hpp. Use this at the unit test runner for libprocess to start getting more valuable valgrind feedback. - Added cleanup of remaining Processes in ~ProcessManager() as a start. Diffs (updated) - 3rdparty/libprocess/include/process/process.hpp 270ca28 3rdparty/libprocess/src/process.cpp d30ed63 3rdparty/libprocess/src/tests/main.cpp 0a3ba4e Diff: https://reviews.apache.org/r/26533/diff/ Testing --- make check support/mesos-style valgrind 3rdparty/libprocess/tests (to see a reduction in definitely lost blocks) Thanks, Joris Van Remoortere
Re: Review Request 26289: Replace some shared_ptr with unique_ptr/Owned to clarify ownership passing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/ --- (Updated Oct. 10, 2014, 2:56 p.m.) Review request for mesos and Jie Yu. Changes --- using unique_ptr where possible. Summary (updated) - Replace some shared_ptr with unique_ptr/Owned to clarify ownership passing. Repository: mesos-git Description --- replace shared_ptr with Owned where ownership is being passed. Diffs (updated) - 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4 3rdparty/libprocess/include/process/defer.hpp ebe6f2db47cab2a3306288d8ebabb720e7cd8976 3rdparty/libprocess/include/process/delay.hpp 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227 3rdparty/libprocess/include/process/event.hpp bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4 07b298f151094e818287f741b3e0efd28374e82b 3rdparty/libprocess/src/process.cpp d30ed636ee24d0fe6e62f69a921307bde1f32765 Diff: https://reviews.apache.org/r/26289/diff/ Testing --- make g++-4.4 and clang++-3.5 make check clang++-3.5 Thanks, Dominic Hamon
Re: Review Request 26571: Fixed ZooKeeper 3.4.5 OSX Yosemite build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/#review56234 --- Till, can you find a shepherd for this issue (@yan ?) and make him the reviewer? - Vinod Kone On Oct. 10, 2014, 7:19 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/ --- (Updated Oct. 10, 2014, 7:19 p.m.) Review request for mesos. Bugs: MESOS-1797 https://issues.apache.org/jira/browse/MESOS-1797 Repository: mesos-git Description --- Fixes build issue caused by htonll introduced by OSX 10.10 (Yosemite). This patch has been submitted upstream as well: https://issues.apache.org/jira/browse/ZOOKEEPER-2049 This patch shall not be committed until upstream has accepted it - I will update this accordingly. Diffs - 3rdparty/zookeeper-3.4.5.patch PRE-CREATION Diff: https://reviews.apache.org/r/26571/diff/ Testing --- make check (OSX 10.10 BETA5) Thanks, Till Toenshoff
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 10, 2014, 6:04 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Till Toenshoff. Changes --- Initialize module manager before everything else. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs (updated) - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26508: Added --module flag for Mesos master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 10, 2014, 6:04 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos master. Diffs - src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/master/flags.hpp b8b59a245523c7e7d4e04f86801ed7b023a129ea src/master/main.cpp 018f8036b742f0b083445e03dca5c06c13d80e68 src/module/manager.hpp 8275fd0c2f0c9ba92a9ad47f7d5d2df9295fa184 Diff: https://reviews.apache.org/r/26508/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 6:04 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Initialize module manager before everything else. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs (updated) - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26274: Remove /proc and /sys remounts from port_mapping isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26274/#review56236 --- Patch looks great! Reviews applied: [26274] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 8:48 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26274/ --- (Updated Oct. 10, 2014, 8:48 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Bugs: MESOS-1853 https://issues.apache.org/jira/browse/MESOS-1853 Repository: mesos-git Description --- Remove /proc and /sys remounts from port_mapping isolator. See ticket for details. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 8ddfb1817d78c45ce2f6daebbeb13d380998ee4d src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b Diff: https://reviews.apache.org/r/26274/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 26529: Modules should accept relative path or file name for library name.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56237 --- Ship it! Ship It! - Niklas Nielsen On Oct. 10, 2014, 2:32 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 10, 2014, 2:32 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Rename path in Modules protobuf to file since a path always referes to an absolute or a relative path, whereas file may refer to a path or a file name. If file contains a slash (/), it is considered an absolute or relative path, otherwise if is considered a file name. For file name, dlopen() automatically does a standard library path search to find the absolute path. Also, return a better error message if an empty module name is provided. Diffs - include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc Diff: https://reviews.apache.org/r/26529/diff/ Testing --- Added tests for LD_LIBRARY_PATH and ran make check. Thanks, Kapil Arya
Review Request 26578: Memory cleanup: libprocess gc finalize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26578/ --- Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Implement the finalize() virtual override for Libprocess' garbage collector. This terminates all processes registered with the garbage collector when the garbage collector terminates. Diffs - 3rdparty/libprocess/include/process/gc.hpp e83c636 Diff: https://reviews.apache.org/r/26578/diff/ Testing --- make check support/mesos-style.py Thanks, Joris Van Remoortere
Review Request 26579: Made the StatusUpdateManager injectable for mocking purposes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26579/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-1799 https://issues.apache.org/jira/browse/MESOS-1799 Repository: mesos-git Description --- This is needed for Vinod's changes in MESOS-1799, where we need a MockStatusUpdateManager for tests. Diffs - src/local/local.cpp b8a481da24419be0e826df26cbc4a4168dd0c3bf src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 src/slave/slave.hpp cb879cb3c953baf0cecace69ed51bc3979d5d81d src/slave/slave.cpp f677adfbf01c0a0c6b6819e3b0165851b0c1366e src/tests/cluster.hpp aad42754fa97c12374a9dda744d20c14ae6b5e61 Diff: https://reviews.apache.org/r/26579/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56244 --- Bad patch! Reviews applied: [26529, 26529] Failed command: git apply --index 26529.patch Error: error: patch failed: include/mesos/module.hpp:27 error: include/mesos/module.hpp: patch does not apply error: patch failed: src/messages/messages.proto:411 error: src/messages/messages.proto: patch does not apply error: patch failed: src/module/manager.cpp:169 error: src/module/manager.cpp: patch does not apply error: patch failed: src/tests/module_tests.cpp:17 error: src/tests/module_tests.cpp: patch does not apply - Mesos ReviewBot On Oct. 10, 2014, 10:04 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 10:04 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 24536: Added DIFF to the replicated log state storage implementation.
On Oct. 1, 2014, 5:45 p.m., Jie Yu wrote: src/Makefile.am, lines 90-91 https://reviews.apache.org/r/24536/diff/3/?file=708154#file708154line90 Ditto my comments on the other review. You might wanna modify configure.ac. Hi Jie, I have a patch after this that changes the configure.ac. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24536/#review55028 --- On Sept. 29, 2014, 12:45 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24536/ --- (Updated Sept. 29, 2014, 12:45 p.m.) Review request for mesos, Ben Mahler and Jie Yu. Repository: mesos-git Description --- See summary. Note that this hard codes the location of the subversion and Apache Portable Runtime (APR) headers. Diffs - src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b src/messages/state.proto 59276e55fcbebdb754c20d39b13b402fd11c3dad src/state/log.cpp fd8b28a0b5d14f5ba3e6fde4695f6d09acf9c56a Diff: https://reviews.apache.org/r/24536/diff/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 26579: Made the StatusUpdateManager injectable for mocking purposes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26579/#review56250 --- Ship it! Ship It! - Vinod Kone On Oct. 10, 2014, 10:16 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26579/ --- (Updated Oct. 10, 2014, 10:16 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-1799 https://issues.apache.org/jira/browse/MESOS-1799 Repository: mesos-git Description --- This is needed for Vinod's changes in MESOS-1799, where we need a MockStatusUpdateManager for tests. Diffs - src/local/local.cpp b8a481da24419be0e826df26cbc4a4168dd0c3bf src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 src/slave/slave.hpp cb879cb3c953baf0cecace69ed51bc3979d5d81d src/slave/slave.cpp f677adfbf01c0a0c6b6819e3b0165851b0c1366e src/tests/cluster.hpp aad42754fa97c12374a9dda744d20c14ae6b5e61 Diff: https://reviews.apache.org/r/26579/diff/ Testing --- make check Thanks, Ben Mahler
Review Request 26580: Memory cleanup: libprocess join worker threads
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26580/ --- Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- During process::finalize() join on the worker threads that were dispatched, after the gc has had a chance to terminate. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26580/diff/ Testing --- make check support/mesos-style.py Thanks, Joris Van Remoortere
Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56251 --- Ship it! Barring that tiny style nit. src/slave/main.cpp https://reviews.apache.org/r/26509/#comment96560 Alphabetize please. src/slave/main.cpp https://reviews.apache.org/r/26509/#comment96561 Not yours but this would trigger a glog warning as it has not yet been initialized. We should possibly check all runnables for such nits at some point. - Till Toenshoff On Oct. 10, 2014, 10:04 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 10:04 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26580: Memory cleanup: libprocess join worker threads
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26580/ --- (Updated Oct. 10, 2014, 11:38 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Changes --- Introduce finalize() on ProcessManager so that we can terminate outstanding processes without destroying our runqueue. Repository: mesos-git Description (updated) --- Terminate the gc; Finalize the ProcessManager (picking up -idealy 0- left-over processes); Join on the worker threads that were dispatched. Diffs (updated) - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26580/diff/ Testing --- make check support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 7:41 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Rebased to latest master. Added endl at the end of error message. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs (updated) - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 7:43 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Fixed #include order Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs (updated) - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26580: Memory cleanup: libprocess join worker threads
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26580/#review56259 --- Patch looks great! Reviews applied: [26578, 26580] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 11:38 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26580/ --- (Updated Oct. 10, 2014, 11:38 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Terminate the gc; Finalize the ProcessManager (picking up -idealy 0- left-over processes); Join on the worker threads that were dispatched. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26580/diff/ Testing --- make check support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 26571: Fixed ZooKeeper 3.4.5 OSX Yosemite build.
On Oct. 10, 2014, 10:01 p.m., Vinod Kone wrote: Till, can you find a shepherd for this issue (@yan ?) and make him the reviewer? Aye, reached out to him on the jira. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/#review56234 --- On Oct. 10, 2014, 7:19 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26571/ --- (Updated Oct. 10, 2014, 7:19 p.m.) Review request for mesos. Bugs: MESOS-1797 https://issues.apache.org/jira/browse/MESOS-1797 Repository: mesos-git Description --- Fixes build issue caused by htonll introduced by OSX 10.10 (Yosemite). This patch has been submitted upstream as well: https://issues.apache.org/jira/browse/ZOOKEEPER-2049 This patch shall not be committed until upstream has accepted it - I will update this accordingly. Diffs - 3rdparty/zookeeper-3.4.5.patch PRE-CREATION Diff: https://reviews.apache.org/r/26571/diff/ Testing --- make check (OSX 10.10 BETA5) Thanks, Till Toenshoff
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56261 --- Bad patch! Reviews applied: [26578, 26580, 26583] Failed command: git apply --index 26583.patch Error: error: patch failed: 3rdparty/libprocess/src/process.cpp:1690 error: 3rdparty/libprocess/src/process.cpp: patch does not apply - Mesos ReviewBot On Oct. 10, 2014, 11:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 11:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26583/#comment96565 this can't land until https://reviews.apache.org/r/26289/ which contains the configure.ac check for std::unique_ptr support. 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26583/#comment96564 should do the same for this while you're here. - Dominic Hamon On Oct. 10, 2014, 4:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 4:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 8:40 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Fixed typos (--module - --modules). Repository: mesos-git Description (updated) --- Added --module flag for Mesos slave. Fixed a type for master flags help message. Diffs (updated) - src/master/flags.hpp 44249f81098dc20d5f13bbfe6bfa89a1bc426cd6 src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Review Request 26588: Fixed small typo in master module flags description.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/ --- Review request for mesos, Kapil Arya and Niklas Nielsen. Repository: mesos-git Description --- see summary. Diffs - src/master/flags.hpp 44249f8 Diff: https://reviews.apache.org/r/26588/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 26588: Fixed small typo in master module flags description.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/ --- (Updated Oct. 11, 2014, 12:41 a.m.) Review request for mesos, Kapil Arya and Niklas Nielsen. Repository: mesos-git Description --- see summary. Diffs - src/master/flags.hpp 44249f8 Diff: https://reviews.apache.org/r/26588/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 26588: Fixed small typo in master module flags description.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/#review56268 --- Ship it! Ship It! - Niklas Nielsen On Oct. 10, 2014, 5:41 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/ --- (Updated Oct. 10, 2014, 5:41 p.m.) Review request for mesos, Kapil Arya and Niklas Nielsen. Repository: mesos-git Description --- see summary. Diffs - src/master/flags.hpp 44249f8 Diff: https://reviews.apache.org/r/26588/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 8:46 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Revert master flag fix. Repository: mesos-git Description (updated) --- Added --module flag for Mesos slave. Diffs (updated) - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26588: Fixed small typo in master module flags description.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/#review56269 --- Ship it! Ship It! - Kapil Arya On Oct. 10, 2014, 8:41 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26588/ --- (Updated Oct. 10, 2014, 8:41 p.m.) Review request for mesos, Kapil Arya and Niklas Nielsen. Repository: mesos-git Description --- see summary. Diffs - src/master/flags.hpp 44249f8 Diff: https://reviews.apache.org/r/26588/diff/ Testing --- Thanks, Till Toenshoff
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56270 --- Bad patch! Reviews applied: [26529] Failed command: git apply --index 26529.patch Error: error: patch failed: include/mesos/module.hpp:27 error: include/mesos/module.hpp: patch does not apply error: patch failed: src/messages/messages.proto:411 error: src/messages/messages.proto: patch does not apply error: patch failed: src/module/manager.cpp:169 error: src/module/manager.cpp: patch does not apply error: patch failed: src/tests/module_tests.cpp:17 error: src/tests/module_tests.cpp: patch does not apply - Mesos ReviewBot On Oct. 11, 2014, 12:46 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 11, 2014, 12:46 a.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 10, 2014, 9:25 p.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Changes --- Removed dependencies. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Re: Review Request 26509: Added --module flag for Mesos slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56274 --- Patch looks great! Reviews applied: [26509] All tests passed. - Mesos ReviewBot On Oct. 11, 2014, 1:25 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 11, 2014, 1:25 a.m.) Review request for mesos, Niklas Nielsen and Till Toenshoff. Repository: mesos-git Description --- Added --module flag for Mesos slave. Diffs - src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d src/slave/main.cpp 1eafb356b58613d4ab8bd6dd245583e0d6f25a97 Diff: https://reviews.apache.org/r/26509/diff/ Testing --- Thanks, Kapil Arya
Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2161
See https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME/2161/changes Changes: [niklas] Added --module flag for Mesos master. [toenshoff] Fixed small typo in master module flags description. -- [...truncated 48412 lines...] I1011 02:59:20.681694 27835 hierarchical_allocator_process.hpp:816] Filtered mem(*):9984; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141011-023120-3142697795-50502-27806-0 for framework 20141011-023120-3142697795-50502-27806- I1011 02:59:20.681725 27835 hierarchical_allocator_process.hpp:659] Performed allocation for 3 slaves in 220748ns I1011 02:59:20.905504 27841 monitor.cpp:140] Failed to collect resource usage for container 'c3378136-2347-414c-a97d-f2055f16a288' for executor 'default' of framework '20141011-023120-3142697795-50502-27806-': Unknown container: c3378136-2347-414c-a97d-f2055f16a288 I1011 02:59:20.90 27841 monitor.cpp:140] Failed to collect resource usage for container 'dda6890c-a863-4f04-80ca-911882bc1be5' for executor 'default' of framework '20141011-023120-3142697795-50502-27806-': Unknown container: dda6890c-a863-4f04-80ca-911882bc1be5 I1011 02:59:21.682534 27840 hierarchical_allocator_process.hpp:816] Filtered cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141011-023120-3142697795-50502-27806-1 for framework 20141011-023120-3142697795-50502-27806- I1011 02:59:21.682623 27840 hierarchical_allocator_process.hpp:816] Filtered mem(*):9984; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141011-023120-3142697795-50502-27806-0 for framework 20141011-023120-3142697795-50502-27806- I1011 02:59:21.682677 27840 hierarchical_allocator_process.hpp:816] Filtered mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):2 on slave 20141011-023120-3142697795-50502-27806-2 for framework 20141011-023120-3142697795-50502-27806- I1011 02:59:21.682708 27840 hierarchical_allocator_process.hpp:659] Performed allocation for 3 slaves in 261425ns I1011 02:59:21.906555 27843 monitor.cpp:140] Failed to collect resource usage for container 'c3378136-2347-414c-a97d-f2055f16a288' for executor 'default' of framework '20141011-023120-3142697795-50502-27806-': Unknown container: c3378136-2347-414c-a97d-f2055f16a288 I1011 02:59:21.906605 27843 monitor.cpp:140] Failed to collect resource usage for container 'dda6890c-a863-4f04-80ca-911882bc1be5' for executor 'default' of framework '20141011-023120-3142697795-50502-27806-': Unknown container: dda6890c-a863-4f04-80ca-911882bc1be5 I1011 02:59:22.408702 27830 slave.cpp:2403] Received ping from slave-observer(1)@67.195.81.187:50502 I1011 02:59:22.683440 27843 hierarchical_allocator_process.hpp:734] Offering mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):2 on slave 20141011-023120-3142697795-50502-27806-2 to framework 20141011-023120-3142697795-50502-27806- I1011 02:59:22.683634 27843 hierarchical_allocator_process.hpp:734] Offering mem(*):9984; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141011-023120-3142697795-50502-27806-0 to framework 20141011-023120-3142697795-50502-27806- I1011 02:59:22.683748 27843 hierarchical_allocator_process.hpp:734] Offering cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] on slave 20141011-023120-3142697795-50502-27806-1 to framework 20141011-023120-3142697795-50502-27806- I1011 02:59:22.683895 27843 hierarchical_allocator_process.hpp:659] Performed allocation for 3 slaves in 621344ns I1011 02:59:22.684124 27843 master.cpp:3729] Sending 3 offers to framework 20141011-023120-3142697795-50502-27806- (Test Framework (C++)) at scheduler-672ea355-894a-4496-b1fa-225e7749d90d@67.195.81.187:50502 Received offer 20141011-023120-3142697795-50502-27806-1007 with cpus(*):2; mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000] Received offer 20141011-023120-3142697795-50502-27806-1008 with mem(*):9984; disk(*):3.70122e+06; ports(*):[31000-32000] Received offer 20141011-023120-3142697795-50502-27806-1009 with mem(*):10240; disk(*):3.70122e+06; ports(*):[31000-32000]; cpus(*):2 I1011 02:59:22.684623 27845 sched.cpp:544] Scheduler::resourceOffers took 93385ns I1011 02:59:22.684958 27841 master.cpp:2315] Processing reply for offers: [ 20141011-023120-3142697795-50502-27806-1007 ] on slave 20141011-023120-3142697795-50502-27806-1 at slave(1)@67.195.81.187:50502 (pomona.apache.org) for framework 20141011-023120-3142697795-50502-27806- (Test Framework (C++)) at scheduler-672ea355-894a-4496-b1fa-225e7749d90d@67.195.81.187:50502 I1011 02:59:22.685118 27841 master.cpp:2315] Processing reply for offers: [ 20141011-023120-3142697795-50502-27806-1008 ] on slave 20141011-023120-3142697795-50502-27806-0 at slave(3)@67.195.81.187:50502 (pomona.apache.org) for framework 20141011-023120-3142697795-50502-27806- (Test Framework (C++)) at
Re: Review Request 26275: MESOS-444: Remove --checkpoint flag
On Oct. 8, 2014, 5:26 p.m., Vinod Kone wrote: Sorry for the delay in committing this. Since we didn't do a proper deprecation, I'm waiting for some of Twitter's clusters to get updated (i.e., removing --checkpoint from slave config files) before landing this on trunk. Let me know if there is an urgency in landing this patch and we'll figure out how to fast track it or do a proper deprecation. IIUC, given that this flag has defaulted to the correct value for a while there shouldn't need to be a delay to commit this patch. If a particular organization's deployment breaks because it's still explicitly specifying this flag it'll find out very quickly at start time right? We can just add a note to UPDATING that users should remove the flag before upgrading to 0.21.0 from 0.20.0. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26275/#review55918 --- On Oct. 6, 2014, 11:50 a.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26275/ --- (Updated Oct. 6, 2014, 11:50 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-444 https://issues.apache.org/jira/browse/MESOS-444 Repository: mesos-git Description --- Checkpointing has been enabled by default in the slave since 0.14, remove the flag now because all slaves should checkpoint. Removing checkpoint from slaves throughout the codebase will occur in a series of following commits. Diffs - src/slave/flags.hpp 16f0cc2 Diff: https://reviews.apache.org/r/26275/diff/ Testing --- make check on ubuntu 14.04 with gcc. Thanks, Cody Maloney
