Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 5:25 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Changes --- Fixed compilation errors; addressed some of Nik's comments. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs (updated) - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 src/slave/main.cpp 2c4d365a04acbcb382e978d811a318130484b3d5 src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 5:30 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Changes --- Do not tie in modules into slave in the first patch. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs (updated) - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/module/manager.hpp, line 88 https://reviews.apache.org/r/25848/diff/1/?file=697028#file697028line88 Can we find some ABI documentation that supports this claim and maybe throw in a reference? Added a TODO for now. On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/module/manager.hpp, lines 117-118 https://reviews.apache.org/r/25848/diff/1/?file=697028#file697028line117 Access to those need to be guarded, right? Or do you explicitally call out that the module manager isn't thread safe? Added a TODO for now. On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/slave/flags.hpp, line 328 https://reviews.apache.org/r/25848/diff/1/?file=697030#file697030line328 Let's tie the modules in after the core patch has landed. Also, we need to it for both masters and slaves (alongside starting to wire up module loading for isolators, authenticators, ...) TL;DR Let's review the src/slave/flags.hpp and src/slave/main.cpp changes (with a master equivalent) in a dependent patch. In the new review, please expand the help text and throw in some examples. You can put in the expected format as you listed in the module manager header. Removed this code for now. Will create a separate patch and address these concerns there. On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/tests/module_tests.cpp, line 123 https://reviews.apache.org/r/25848/diff/1/?file=697033#file697033line123 Where are we going to capture the negative tests? Wrong library versions, trying to load non-mesos modules libraries (libc.so for example)? Added a TODO for now. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54043 --- On Sept. 22, 2014, 5:30 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 5:30 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54118 --- Bad patch! Reviews applied: [25848] Failed command: ./support/mesos-style.py Error: Checking 513 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo src/module/manager.hpp:85: Redundant blank line at the end of a code block should be deleted. [whitespace/blank_line] [3] Total errors found: 1 - Mesos ReviewBot On Sept. 22, 2014, 9:30 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 9:30 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 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 Sept. 22, 2014, 1:10 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till Toenshoff. Changes --- Add auto usage examples; rebase. Bugs: MESOS-1793 https://issues.apache.org/jira/browse/MESOS-1793 Repository: mesos-git Description (updated) --- Explicitly prohibit the use of namespace aliases. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.
On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote: src/exec/exec.cpp, line 82 https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line82 Maybe I'm missing something, but is there a reason we don't check before a delay? If ShutdownProcess is spawned, it's possible all other processes in the group have already cleaned up. Alexander Rukletsov wrote: Good questions. Can this be related to non-native bindings? Timothy St. Clair wrote: Perhaps we can open a separate JIRA on this issue as it was here before. https://issues.apache.org/jira/browse/MESOS-1823 - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review53038 --- On Sept. 18, 2014, 11:03 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Sept. 18, 2014, 11:03 a.m.) Review request for mesos, Benjamin Hindman, Niklas Nielsen, Till Toenshoff, and Timothy St. Clair. Bugs: MESOS-1571 https://issues.apache.org/jira/browse/MESOS-1571 Repository: mesos-git Description --- The configurable slave's executor_shutdown_grace_period flag is propagated to Executor and CommandExecutor through an environment variable. Shutdown timeout in Executor and signal escalation timeout in CommandExecutor are now dependent on this flag. Each nested timeout is somewhat shorter than the parent one. Diffs - src/Makefile.am 9b973e5 src/exec/exec.cpp e15f834 src/launcher/executor.cpp cbc8750 src/slave/constants.hpp 9030871 src/slave/constants.cpp e1da5c0 src/slave/containerizer/containerizer.hpp 8a66412 src/slave/containerizer/containerizer.cpp 0254679 src/slave/containerizer/docker.cpp 0febbac src/slave/containerizer/external_containerizer.cpp efbc68f src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/slave/flags.hpp 21e0021 src/slave/utils.hpp PRE-CREATION src/slave/utils.cpp PRE-CREATION src/tests/containerizer.cpp a17e1e0 Diff: https://reviews.apache.org/r/25434/diff/ Testing --- make check (OS X 10.9.4) 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/#review54127 --- Patch looks great! Reviews applied: [25622] All tests passed. - Mesos ReviewBot On Sept. 22, 2014, 1:10 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 1:10 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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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: Mesos Modules Design
Folks, I think this thread has fallen off the rails a bit, perhaps we could get a tcon/hangout going for this again. I would be happy to moderate if needed, but it's pretty clear to me to that trying to hash this out in an email thread is becoming couter-productive. Cheers, Tim - Original Message - From: Dominic Hamon dha...@twitter.com.INVALID To: dev dev@mesos.apache.org Sent: Saturday, September 20, 2014 9:21:59 AM Subject: Re: Mesos Modules Design On Fri, Sep 19, 2014 at 2:43 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Dominic, (response inlined) On 19 September 2014 13:03, Dominic Hamon dha...@twopensource.com wrote: I'm sorry, but I'm still having a hard time understanding why this needs to be dynamic. If the mesos core is split into modules that are built as standalone libraries (static) then at link time the right combination of libraries can be bundled together to create the end result. If you want to get even smarter, we can have default versions that are linked in to the mesos core as weak symbols so later linked libraries can override the defaults. This may mean that we move to static linking across the board, but frankly there are a few benefits to that approach. This works for some, but not all use cases. One use-case where it _does_ make sense to statically bake into the image could be: https://issues.apache.org/jira/browse/MESOS-1330 That be, you probably rarely want to swap network transport implementations in and out on per-run basis but will know up front which one to configure and use. On the other hand, having to relink and rebuilding give users a poor experience and makes it hard to select (or unselect) custom components. Using a prebuilt package and point against libraries is a pretty common work-flow: Apache web server relies heavily on modules from dynamic libraries. I'm going to argue against this, so please bear with me while I work through it, and please point out anywhere that my assertions are wrong. The proposal you are suggesting requires the following: - callsites need to be modules aware to use the right factory method to instantiate the modular object - mesos-core owners need to be aware of modules and versioning when they change the abstract API - module owners need to be aware of the modules API and versioning - customers (end-users) have to be modules aware and set their runtime configuration correctly - errors in versioning or modules will only be caught at runtime My alternative proposal requires the following: - module owners need to be aware of API changes during core upgrades - issues are caught at compile time by module owners - customers (end-users) are unaware of modules and only need to know about per module configuration (kerberos attributes, for instance) not the modules system and configuration In short, your proposal adds more complexity and shifts the burden of knowledge and responsibility to every agent, instead of keeping it with the module owner where it belongs. I think the solution that's been put up for review is clever, but I don't think it solves the problem in the right way for anyone involved. Please rethink your approach to take that into account. TL;DR I don't see the two approaches to be mutually exclusive and we can get a lot of leverage with the current design but we want to be able to do this with static linking too. (To Tim St Clair's point) With the approach as defined, does this mean that the default versions will also have to be reimplemented as modules? Not necessary. Mesos default implementations stays as is. See Bernd's comment. Has any effort been put into determining the performance overhead of the approach as specified? It won't affect Mesos if you are not using modules. The call sites will be virtual dispatches no matter whether you are using modules or internal/default implementations. There is a performance penalty of not being able to do global optimizations within the module, but that is the trade-off of implementing a dynamic loadable module. On Fri, Sep 19, 2014 at 11:35 AM, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and https://issues.apache.org/jira/browse/MESOS-1384. Kapil and Bernd took over the work (shepherded by Ben H and I) and have expanded on the original design to cope with api/modules/mesos versioning semantics and be extensible enough to cope future changes in the modules subsystem (dealing with modules dependencies, etc).
Auto usage in mesos
We now start using auto in the code (among several other C++11 features). However we don't want to hamper reasoning about types. I would suggest we select several use cases where we all agree using auto is welcomed and several counterexamples. After the short conversation with BenH, we agreed that iterators on one side and Try, Option on the other side are first candidates for such list. I put the examples here https://reviews.apache.org/r/25622/. Any thoughts and ideas?
Re: Mesos Modules Design
Agreed - I can do early morning PST any day during this week (for people dialing in from the east coast and Europe). Niklas On 22 September 2014 07:15, Tim St Clair tstcl...@redhat.com wrote: Folks, I think this thread has fallen off the rails a bit, perhaps we could get a tcon/hangout going for this again. I would be happy to moderate if needed, but it's pretty clear to me to that trying to hash this out in an email thread is becoming couter-productive. Cheers, Tim - Original Message - From: Dominic Hamon dha...@twitter.com.INVALID To: dev dev@mesos.apache.org Sent: Saturday, September 20, 2014 9:21:59 AM Subject: Re: Mesos Modules Design On Fri, Sep 19, 2014 at 2:43 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Dominic, (response inlined) On 19 September 2014 13:03, Dominic Hamon dha...@twopensource.com wrote: I'm sorry, but I'm still having a hard time understanding why this needs to be dynamic. If the mesos core is split into modules that are built as standalone libraries (static) then at link time the right combination of libraries can be bundled together to create the end result. If you want to get even smarter, we can have default versions that are linked in to the mesos core as weak symbols so later linked libraries can override the defaults. This may mean that we move to static linking across the board, but frankly there are a few benefits to that approach. This works for some, but not all use cases. One use-case where it _does_ make sense to statically bake into the image could be: https://issues.apache.org/jira/browse/MESOS-1330 That be, you probably rarely want to swap network transport implementations in and out on per-run basis but will know up front which one to configure and use. On the other hand, having to relink and rebuilding give users a poor experience and makes it hard to select (or unselect) custom components. Using a prebuilt package and point against libraries is a pretty common work-flow: Apache web server relies heavily on modules from dynamic libraries. I'm going to argue against this, so please bear with me while I work through it, and please point out anywhere that my assertions are wrong. The proposal you are suggesting requires the following: - callsites need to be modules aware to use the right factory method to instantiate the modular object - mesos-core owners need to be aware of modules and versioning when they change the abstract API - module owners need to be aware of the modules API and versioning - customers (end-users) have to be modules aware and set their runtime configuration correctly - errors in versioning or modules will only be caught at runtime My alternative proposal requires the following: - module owners need to be aware of API changes during core upgrades - issues are caught at compile time by module owners - customers (end-users) are unaware of modules and only need to know about per module configuration (kerberos attributes, for instance) not the modules system and configuration In short, your proposal adds more complexity and shifts the burden of knowledge and responsibility to every agent, instead of keeping it with the module owner where it belongs. I think the solution that's been put up for review is clever, but I don't think it solves the problem in the right way for anyone involved. Please rethink your approach to take that into account. TL;DR I don't see the two approaches to be mutually exclusive and we can get a lot of leverage with the current design but we want to be able to do this with static linking too. (To Tim St Clair's point) With the approach as defined, does this mean that the default versions will also have to be reimplemented as modules? Not necessary. Mesos default implementations stays as is. See Bernd's comment. Has any effort been put into determining the performance overhead of the approach as specified? It won't affect Mesos if you are not using modules. The call sites will be virtual dispatches no matter whether you are using modules or internal/default implementations. There is a performance penalty of not being able to do global optimizations within the module, but that is the trade-off of implementing a dynamic loadable module. On Fri, Sep 19, 2014 at 11:35 AM, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and
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 Sept. 22, 2014, 4:25 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Changes --- Explain why we prefer TASK_KILLED and not sending the update. 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 e5d30e9 src/tests/master_tests.cpp 8e4ec1d 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 Sept. 15, 2014, 4:38 p.m., Benjamin Hindman wrote: src/master/master.cpp, line 4010 https://reviews.apache.org/r/25250/diff/4/?file=682254#file682254line4010 I suggest we use TASK_LOST here instead. We definitely want a terminal state like TASK_KILLED, but we've reserved TASK_KILLED for when a framework has actually intiated the kill itself, and thus I'd prefer not to overload the semantics. This might be a good candidate for a new task state, e.g., TASK_REMOVED, which has been discussed in the past, but I can't recall if there is a JIRA for that or not. If not, it would be great to have you create one Alex so we can have a discussion about how to introduce new task states (and maybe even a way to introduce sub-states that framework writers themselves could customize). Alexander Rukletsov wrote: I used to have `TASK_LOST` here, but my understanding is that `TASK_LOST` is used for abnormal situations, i.e. when the task is not finished not because of scheduler's direct command, but because of some external reasons. I agree, that a new task state is a very good solution. We have [this ticket](https://issues.apache.org/jira/browse/MESOS-343), one solution for which would be to introduce something like `TaskStatusExplained` or a protobuf message for every task. But maybe for this situation something like `TASK_ABANDONED` would be rather descriptive. Keeping TASK_KILLED, but added comments for clarity. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review53350 --- On Sept. 22, 2014, 4:25 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 22, 2014, 4:25 p.m.) Review request for mesos, Benjamin Hindman 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 e5d30e9 src/tests/master_tests.cpp 8e4ec1d Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54125 --- Thanks Jie! 1st pass testing worked, but I need to beat on it some more. Normally I would say we should probably separate out the cleanup work from the feature mod, but it's not that important to me. src/linux/cgroups.cpp https://reviews.apache.org/r/25858/#comment94124 I think I've missed something subtle, how did you bypass the cleanup semantics? src/slave/containerizer/isolators/cgroups/cpushare.cpp https://reviews.apache.org/r/25858/#comment94126 src/slave/containerizer/isolators/cgroups/cpushare.cpp https://reviews.apache.org/r/25858/#comment94127 So does this rely on realpath squashing the symbolic link such that they are = ? - Timothy St. Clair On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
On Sept. 22, 2014, 4:31 p.m., Timothy St. Clair wrote: src/linux/cgroups.cpp, line 1795 https://reviews.apache.org/r/25858/diff/1/?file=698357#file698357line1795 I think I've missed something subtle, how did you bypass the cleanup semantics? The isolators won't try to unmount and rm the hierarchies (i.e., it won't call cgroups::cleanup). The only place we call cgroups::cleanup is in tests, I will do a sweep today to make sure we don't accidentally umount systemd managed hierarchies. On Sept. 22, 2014, 4:31 p.m., Timothy St. Clair wrote: src/slave/containerizer/isolators/cgroups/cpushare.cpp, line 112 https://reviews.apache.org/r/25858/diff/1/?file=698359#file698359line112 So does this rely on realpath squashing the symbolic link such that they are = ? In fact, we obtain the hierarchy from /proc/mounts. Yes, we also call realpath. So I think this part is safe. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54125 --- On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
On Sept. 22, 2014, 4:31 p.m., Timothy St. Clair wrote: Thanks Jie! 1st pass testing worked, but I need to beat on it some more. Normally I would say we should probably separate out the cleanup work from the feature mod, but it's not that important to me. Sorry about that. I should have separated this patch:) - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54125 --- On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Auto usage in mesos
For reference: http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#auto We should be able to adopt that wholesale but please document anywhere you think we would diverge from those examples. On Mon, Sep 22, 2014 at 7:28 AM, Alex Rukletsov a...@mesosphere.io wrote: We now start using auto in the code (among several other C++11 features). However we don't want to hamper reasoning about types. I would suggest we select several use cases where we all agree using auto is welcomed and several counterexamples. After the short conversation with BenH, we agreed that iterators on one side and Try, Option on the other side are first candidates for such list. I put the examples here https://reviews.apache.org/r/25622/. Any thoughts and ideas? -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 12:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 19, 2014, 5:58 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 19, 2014, 5:58 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
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/#review54140 --- docs/mesos-c++-style-guide.md https://reviews.apache.org/r/25622/#comment94131 you may need to rebase this .. i added this list and included variadic templates. - Dominic Hamon On Sept. 22, 2014, 6:10 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 6:10 a.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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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: Mesos Modules Design
+1. On Sep 22, 2014, at 5:22 PM, Niklas Nielsen nik...@mesosphere.io wrote: Agreed - I can do early morning PST any day during this week (for people dialing in from the east coast and Europe). Niklas On 22 September 2014 07:15, Tim St Clair tstcl...@redhat.com wrote: Folks, I think this thread has fallen off the rails a bit, perhaps we could get a tcon/hangout going for this again. I would be happy to moderate if needed, but it's pretty clear to me to that trying to hash this out in an email thread is becoming couter-productive. Cheers, Tim - Original Message - From: Dominic Hamon dha...@twitter.com.INVALID To: dev dev@mesos.apache.org Sent: Saturday, September 20, 2014 9:21:59 AM Subject: Re: Mesos Modules Design On Fri, Sep 19, 2014 at 2:43 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi Dominic, (response inlined) On 19 September 2014 13:03, Dominic Hamon dha...@twopensource.com wrote: I'm sorry, but I'm still having a hard time understanding why this needs to be dynamic. If the mesos core is split into modules that are built as standalone libraries (static) then at link time the right combination of libraries can be bundled together to create the end result. If you want to get even smarter, we can have default versions that are linked in to the mesos core as weak symbols so later linked libraries can override the defaults. This may mean that we move to static linking across the board, but frankly there are a few benefits to that approach. This works for some, but not all use cases. One use-case where it _does_ make sense to statically bake into the image could be: https://issues.apache.org/jira/browse/MESOS-1330 That be, you probably rarely want to swap network transport implementations in and out on per-run basis but will know up front which one to configure and use. On the other hand, having to relink and rebuilding give users a poor experience and makes it hard to select (or unselect) custom components. Using a prebuilt package and point against libraries is a pretty common work-flow: Apache web server relies heavily on modules from dynamic libraries. I'm going to argue against this, so please bear with me while I work through it, and please point out anywhere that my assertions are wrong. The proposal you are suggesting requires the following: - callsites need to be modules aware to use the right factory method to instantiate the modular object - mesos-core owners need to be aware of modules and versioning when they change the abstract API - module owners need to be aware of the modules API and versioning - customers (end-users) have to be modules aware and set their runtime configuration correctly - errors in versioning or modules will only be caught at runtime My alternative proposal requires the following: - module owners need to be aware of API changes during core upgrades - issues are caught at compile time by module owners - customers (end-users) are unaware of modules and only need to know about per module configuration (kerberos attributes, for instance) not the modules system and configuration In short, your proposal adds more complexity and shifts the burden of knowledge and responsibility to every agent, instead of keeping it with the module owner where it belongs. I think the solution that's been put up for review is clever, but I don't think it solves the problem in the right way for anyone involved. Please rethink your approach to take that into account. TL;DR I don't see the two approaches to be mutually exclusive and we can get a lot of leverage with the current design but we want to be able to do this with static linking too. (To Tim St Clair's point) With the approach as defined, does this mean that the default versions will also have to be reimplemented as modules? Not necessary. Mesos default implementations stays as is. See Bernd's comment. Has any effort been put into determining the performance overhead of the approach as specified? It won't affect Mesos if you are not using modules. The call sites will be virtual dispatches no matter whether you are using modules or internal/default implementations. There is a performance penalty of not being able to do global optimizations within the module, but that is the trade-off of implementing a dynamic loadable module. On Fri, Sep 19, 2014 at 11:35 AM, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, We have been iterating on a design for pluggable modules in Mesos lately and wanted to get a last round of feedback before putting out patch sets. Tim St Clair, Ben Hindman and I started the discussion (and work) on this subsystem https://issues.apache.org/jira/browse/MESOS-1224 and https://issues.apache.org/jira/browse/MESOS-1384. Kapil and Bernd took over the work (shepherded by Ben H and I) and have expanded
Re: Review Request 25789: Variadic strings join
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789/ --- (Updated Sept. 22, 2014, 5:30 p.m.) Review request for mesos and Benjamin Hindman. Changes --- Remove the for c++11 and above comments since variadic templates are now part of the configuration check. Summary (updated) - Variadic strings join Repository: mesos-git Description (updated) --- Add Variadic strings join. There is a second version of the variadic join which takes a reference to a stringstream as a parameter. This is handy when strings::join is just a part of a larger string manipulation. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25789/diff/ Testing --- Ran make check for stout. Added test cases for join as these were missing. Thanks, Joris Van Remoortere
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54141 --- Ship it! src/linux/cgroups.cpp https://reviews.apache.org/r/25858/#comment94132 Not your code, but it's a little silly to do this every time... src/slave/containerizer/isolators/cgroups/cpushare.hpp https://reviews.apache.org/r/25858/#comment94137 I don't think it's exclusively systems using systemd. What about, (e.g., systems using systemd) ? src/slave/containerizer/isolators/cgroups/cpushare.cpp https://reviews.apache.org/r/25858/#comment94138 So cgroups::destroy gets called with col-located hierarchies? Do we not need to leave this to systemd to clean up? src/slave/slave.cpp https://reviews.apache.org/r/25858/#comment94133 keep the NOTE on a new line? - Ian Downes On Sept. 19, 2014, 5:21 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 19, 2014, 5:21 p.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 7:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. Niklas Nielsen wrote: It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. Hi Ben, I am refactoring certain classes from process.cpp into the include directory because my goal is to allow pluggable implementations (modules) of the event-management abstraction. My thought process is that we should have the classes required to implement this abstraction available in the public include dir. Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled. I am in the process of making a branch available as a prototype for MESOS-1330; which will make it more apparent where I am going. What are your thoughts on the rules behind the public include dir, and what requirements we can impose on module implementers? - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 20, 2014, 12:58 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 20, 2014, 12:58 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Auto usage in mesos
There are some cases where using auto for function return type deduction is necessary to be able to state what you want generically in templates, and I think we should allow that use case. I can ping some friends for good examples if people would like. Definitely general use of return type deduction shouldn't happen, but inside templates a lot of the time the code is a lot simpler, more readable, and easier to maintain when using auto (Esp. c++14 return type deduction auto), than if a type has to be hand-deduced. Also note at times we just can't say what the type is, because it varies in the template instantiation. See my use in https://reviews.apache.org/r/25525/diff/, slaveinfo_utils.cpp where writing explicit types for memberFunc is really hard if not impossible. On Mon, Sep 22, 2014 at 9:57 AM, Dominic Hamon dha...@twopensource.com wrote: For reference: http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#auto We should be able to adopt that wholesale but please document anywhere you think we would diverge from those examples. On Mon, Sep 22, 2014 at 7:28 AM, Alex Rukletsov a...@mesosphere.io wrote: We now start using auto in the code (among several other C++11 features). However we don't want to hamper reasoning about types. I would suggest we select several use cases where we all agree using auto is welcomed and several counterexamples. After the short conversation with BenH, we agreed that iterators on one side and Try, Option on the other side are first candidates for such list. I put the examples here https://reviews.apache.org/r/25622/. Any thoughts and ideas? -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
Twitter sprint Q3.6
Hello Here's our focus for the next two weeks. Some of these we are shepherding, but are high enough profile that we want to track them. Link to active sprint https://issues.apache.org/jira/secure/RapidBoard.jspa?rapidView=35 Task reconciliation: - Removing out-of-order updates Isolation: - /tmp isolation - PID namespaces - Expose RTT in container stats Persistence: - Responding to design comments Reliability: - Handling temporary one-way network splits - Access to stats.json after framework stops - Allow slave reconfiguration on restart On Tue, Sep 9, 2014 at 10:16 AM, Dominic Hamon dha...@twopensource.com wrote: Hello Mesos developers! I thought it might be useful to have a little insight into how we work on Mesos at Twitter and, more specifically, what we are working on. We work in biweekly sprints and starting with this one (that just began yesterday) I'm going to start sending out a summary of what we're planning. I hope you find it useful; feedback is encouraged. Link to active sprint https://issues.apache.org/jira/secure/RapidBoard.jspa?rapidView=35 Task reconciliation: - extending master state to include terminal unacknowledged tasks - adding pending tasks to slave endpoints - send pending tasks during reregistration of slave with master Isolation: - replacing freezer with PID namespaces to avoid lost tasks during container destruction - /tmp isolation Reliability: - handling race between znode removal and state read - allowing increase in available resources without requiring slave drain - dominic -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
On Sept. 22, 2014, 5:33 p.m., Ian Downes wrote: src/linux/cgroups.cpp, line 517 https://reviews.apache.org/r/25858/diff/1/?file=698357#file698357line517 Not your code, but it's a little silly to do this every time... Added a TODO. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54141 --- On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Review Request 25861: Serialize isolator prepare and cleanup (reversed).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25861/ --- Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup). Diffs - src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c Diff: https://reviews.apache.org/r/25861/diff/ Testing --- make check Thanks, Ian Downes
Review Request 25863: Rename stout/os/setns.hpp to namespaces.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25863/ --- Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Also, added os::getns(pid, ns) to get the namespace inode for comparisons between pids' namespaces. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am db9766d70adb9076946cd2b467c55636fe5f7235 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de53c3873ecd0b62a08ca9aac530043ffb9 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6fa5b741bdd7f089ba93bf6fea43b9f39f8f0edb 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp 5278996f201a4a3d69282c1bd7b0d230d0f6cd39 3rdparty/libprocess/3rdparty/stout/tests/os/namespaces_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp ad8e37aa2f5a29f8b421dde6b7cd5dfe241eabb5 src/slave/containerizer/isolators/network/port_mapping.cpp 9248460caa558e43a2a7d237f6a37d43f0eeacb5 Diff: https://reviews.apache.org/r/25863/diff/ Testing --- Added test to check a clone(NEW_PIDNS) results in a new pid namespace. Thanks, Ian Downes
Review Request 25864: Add 'FutureNothing cgroups::empty()'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/ --- Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Polls cgroups.procs until no processes in the cgroup. Poll interval and timeout can be specified. Diffs - src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 Diff: https://reviews.apache.org/r/25864/diff/ Testing --- Thanks, Ian Downes
Review Request 25862: Improve shared filesystem isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25862/ --- Review request for mesos and Vinod Kone. Repository: mesos-git Description --- * Ensure SharedFilesystemIsolator is the first isolator in prepare (and last in cleanup). * Remounts /proc and /sys for the container. * Remove /proc and /sys remounts from PortMappingIsolator. * Make the SharedFilesystemIsolator a dependency for the PortMappingIsolator. Diffs - src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/isolators/network/port_mapping.cpp 9248460caa558e43a2a7d237f6a37d43f0eeacb5 src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c Diff: https://reviews.apache.org/r/25862/diff/ Testing --- make check Thanks, Ian Downes
Re: Review Request 25549: Basic filesystem isolator for Linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/ --- (Updated Sept. 22, 2014, 11:45 a.m.) Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. Changes --- Addressed comments. Bugs: MESOS-1586 https://issues.apache.org/jira/browse/MESOS-1586 Repository: mesos-git Description --- Does not report usage or enforce quota but can create 'private' directories for each container which mask parts of the shared host filesystem. This review replaces https://reviews.apache.org/r/24178/ because of some file renaming. I addressed all comments from earlier reviews. Diffs (updated) - include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 Diff: https://reviews.apache.org/r/25549/diff/ Testing --- make check # added a test Thanks, Ian Downes
Review Request 25865: Pid namespace isolator for the MesosContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/ --- Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace. Diffs - src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 Diff: https://reviews.apache.org/r/25865/diff/ Testing --- Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc). Thanks, Ian Downes
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54165 --- Ship it! Ship It! - Timothy St. Clair On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54164 --- src/linux/cgroups.cpp https://reviews.apache.org/r/25858/#comment94159 Failed to check existence of the nested test cgroup - Timothy Chen On Sept. 20, 2014, 12:21 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 20, 2014, 12:21 a.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25864: Add 'FutureNothing cgroups::empty()'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/#review54166 --- src/linux/cgroups.cpp https://reviews.apache.org/r/25864/#comment94161 Since you have multiple callers now, can you include in the log what operation that was timed out? - Timothy Chen On Sept. 22, 2014, 6:46 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/ --- (Updated Sept. 22, 2014, 6:46 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Polls cgroups.procs until no processes in the cgroup. Poll interval and timeout can be specified. Diffs - src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 Diff: https://reviews.apache.org/r/25864/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 25862: Improve shared filesystem isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25862/#review54167 --- src/slave/containerizer/linux_launcher.cpp https://reviews.apache.org/r/25862/#comment94162 Return error if pid not exists? - Timothy Chen On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25862/ --- (Updated Sept. 22, 2014, 6:45 p.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- * Ensure SharedFilesystemIsolator is the first isolator in prepare (and last in cleanup). * Remounts /proc and /sys for the container. * Remove /proc and /sys remounts from PortMappingIsolator. * Make the SharedFilesystemIsolator a dependency for the PortMappingIsolator. Diffs - src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/isolators/network/port_mapping.cpp 9248460caa558e43a2a7d237f6a37d43f0eeacb5 src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c Diff: https://reviews.apache.org/r/25862/diff/ Testing --- make check Thanks, Ian Downes
Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/#review54169 --- src/slave/containerizer/isolators/namespaces/pid.cpp https://reviews.apache.org/r/25865/#comment94166 Might be missing something of how pid namespaces work, but I don't see where we clean up the BIND_MOUNT_ROOT. Why do we have to (or can) assume this holder doesn't exist on launch? - Timothy Chen On Sept. 22, 2014, 6:46 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/ --- (Updated Sept. 22, 2014, 6:46 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace. Diffs - src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 Diff: https://reviews.apache.org/r/25865/diff/ Testing --- Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc). Thanks, Ian Downes
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 7:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. Niklas Nielsen wrote: It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. Joris Van Remoortere wrote: Hi Ben, I am refactoring certain classes from process.cpp into the include directory because my goal is to allow pluggable implementations (modules) of the event-management abstraction. My thought process is that we should have the classes required to implement this abstraction available in the public include dir. Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled. I am in the process of making a branch available as a prototype for MESOS-1330; which will make it more apparent where I am going. What are your thoughts on the rules behind the public include dir, and what requirements we can impose on module implementers? For the public include directory, if the abstraction is generally useful (e.g. Node, Socket is less clearly useful but looks ok) then it's fine to include these in the public headers. `ProcessReference` however, is not something that the library user can leverage, and it's dangerous to expose: use of it can lead to ProcessManager::cleanup spinning forever! _My thought process is that we should have the classes required to implement this abstraction available in the public include dir._ Are you interested in applying the same philosophy to the mesos code base? That would grow the public headers substantially. _Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled._ Seems like a reasonable thing for module implementers IMHO, otherwise we may have to expose a lot of internals in the public include folder of both libprocess and mesos. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 20, 2014, 12:58 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 20, 2014, 12:58 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25864: Add 'FutureNothing cgroups::empty()'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/#review54170 --- Why do you need this? Seems like one would only watch for cgroup emptiness when destroying a cgroup, in which case, why split emptiness waiting from a successful destroy? - Ben Mahler On Sept. 22, 2014, 6:46 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/ --- (Updated Sept. 22, 2014, 6:46 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Polls cgroups.procs until no processes in the cgroup. Poll interval and timeout can be specified. Diffs - src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 Diff: https://reviews.apache.org/r/25864/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 25785: Add guide to becoming a committer
On Sept. 19, 2014, 5:24 p.m., Adam B wrote: I think it's valuable to outline some guidelines for anybody interested in becoming a committer, but I don't think we should set too many strict rules and regulations around it. Other thoughts below. I think exactly the opposite. I think clear guidelines give contributors goals to attain and allow them to have an aim further than just contributing to the project. It also reduces friction when deciding who to nominate as a new committer and encourages the nomination of new committers as contributors mature. On Sept. 19, 2014, 5:24 p.m., Adam B wrote: docs/becoming-a-committer.md, line 13 https://reviews.apache.org/r/25785/diff/1/?file=693607#file693607line13 10-20 non-trivial patches seems arbitrarily high. I've seen us call votes for contributors with as few as a dozen patches, and some of them may have been trivial by some definition. We should discuss what the actual (non-trivial) patch limit should be. I would be fine nominating somebody with 5 non-trivial patches if they are active in the community and committed to the project. If they are to be a docs or website committer, the requirements could be even less. Maybe we shouldn't even publish a hard limit publicly. Maybe anybody can ask to be nominated, and the existing committers can evaluate the requirements for that individual/role. I think a limit or guideline is important to avoid the decision-making seem arbitrary, however i'm completely open to changing this number based on what people are comfortable with. There's always going to be room for interpretation when actually voting regarding the triviality or otherwise of the patches. On Sept. 19, 2014, 5:24 p.m., Adam B wrote: docs/becoming-a-committer.md, line 27 https://reviews.apache.org/r/25785/diff/1/?file=693607#file693607line27 Why 10 days? Why not 1 week? How did you arrive at this number? I had to start somewhere. I wanted something that would give people enough time to vote and as people often go on vacation for a week, it avoids people missing the chance to give feedback. On Sept. 19, 2014, 5:24 p.m., Adam B wrote: docs/becoming-a-committer.md, line 34 https://reviews.apache.org/r/25785/diff/1/?file=693607#file693607line34 Does that mean we will be revoking committer status from the Berkeley guys who have moved on to other things and are no longer contributing to Mesos? i need to find better wording for this. by 'positive contributor' i only meant 'not a negative contributor'. ie, not working actively against the project or being difficult to work with. On Sept. 19, 2014, 5:24 p.m., Adam B wrote: docs/becoming-a-committer.md, line 36 https://reviews.apache.org/r/25785/diff/1/?file=693607#file693607line36 Has this process been used/defined previously, or are you proposing a new process here? How long does the revocation vote last? I believe this is a new process. - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/#review54053 --- On Sept. 18, 2014, 10:41 a.m., Dominic Hamon wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25785/ --- (Updated Sept. 18, 2014, 10:41 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-1815 https://issues.apache.org/jira/browse/MESOS-1815 Repository: mesos-git Description --- Add a guide to becoming a committer. Diffs - docs/becoming-a-committer.md PRE-CREATION Diff: https://reviews.apache.org/r/25785/diff/ Testing --- Thanks, Dominic Hamon
Re: Review Request 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/#review54173 --- docs/mesos-c++-style-guide.md https://reviews.apache.org/r/25622/#comment94168 Why would the iterator be called `containerizer`? s/containerizer/iterator/ ? docs/mesos-c++-style-guide.md https://reviews.apache.org/r/25622/#comment94170 Will work with gcc 4.4+? :) - Ben Mahler On Sept. 22, 2014, 1:10 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 1:10 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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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 25622: Update the Mesos Style Guide with C++11 and naming notes.
On Sept. 22, 2014, 12:19 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, line 104 https://reviews.apache.org/r/25622/diff/3/?file=699636#file699636line104 Will work with gcc 4.4+? :) yes. it's in the configure check :) - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review54173 --- On Sept. 22, 2014, 6:10 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 6:10 a.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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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 25622: Update the Mesos Style Guide with C++11 and naming notes.
On Sept. 22, 2014, 12:19 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, lines 96-99 https://reviews.apache.org/r/25622/diff/3/?file=699636#file699636line96 Why would the iterator be called `containerizer`? s/containerizer/iterator/ ? -1 naming a variable after a type is never a good idea. in this case, you're getting a containerizer (iterator) from the container of containerizers so the name 'containerizer' makes sense. - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review54173 --- On Sept. 22, 2014, 6:10 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 6:10 a.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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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 25622: Update the Mesos Style Guide with C++11 and naming notes.
On Sept. 22, 2014, 7:19 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, lines 96-99 https://reviews.apache.org/r/25622/diff/3/?file=699636#file699636line96 Why would the iterator be called `containerizer`? s/containerizer/iterator/ ? Dominic Hamon wrote: -1 naming a variable after a type is never a good idea. in this case, you're getting a containerizer (iterator) from the container of containerizers so the name 'containerizer' makes sense. Sounds confusing. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review54173 --- On Sept. 22, 2014, 1:10 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 22, 2014, 1:10 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. The discussion about using namespace aliases took place in [the other review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed not to introduce them in code. 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 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 7:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. Niklas Nielsen wrote: It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. Joris Van Remoortere wrote: Hi Ben, I am refactoring certain classes from process.cpp into the include directory because my goal is to allow pluggable implementations (modules) of the event-management abstraction. My thought process is that we should have the classes required to implement this abstraction available in the public include dir. Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled. I am in the process of making a branch available as a prototype for MESOS-1330; which will make it more apparent where I am going. What are your thoughts on the rules behind the public include dir, and what requirements we can impose on module implementers? Ben Mahler wrote: For the public include directory, if the abstraction is generally useful (e.g. Node, Socket is less clearly useful but looks ok) then it's fine to include these in the public headers. `ProcessReference` however, is not something that the library user can leverage, and it's dangerous to expose: use of it can lead to ProcessManager::cleanup spinning forever! _My thought process is that we should have the classes required to implement this abstraction available in the public include dir._ Are you interested in applying the same philosophy to the mesos code base? That would grow the public headers substantially. _Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled._ Seems like a reasonable thing for module implementers IMHO, otherwise we may have to expose a lot of internals in the public include folder of both libprocess and mesos. Thanks for your feedback Ben. I'm fine with requiring module implementers to pull the src; I just did not want to force the issue without a discussion. If everyone else is ok with this then I will keep things in the src dirs. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 20, 2014, 12:58 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 20, 2014, 12:58 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.
On Sept. 22, 2014, 7:19 p.m., Ben Mahler wrote:
docs/mesos-c++-style-guide.md, lines 96-99
https://reviews.apache.org/r/25622/diff/3/?file=699636#file699636line96
Why would the iterator be called `containerizer`?
s/containerizer/iterator/ ?
Dominic Hamon wrote:
-1
naming a variable after a type is never a good idea. in this case, you're
getting a containerizer (iterator) from the container of containerizers so
the name 'containerizer' makes sense.
Ben Mahler wrote:
Sounds confusing.
If 'auto' was not used here, would we call this 'containerizer'? In a loop,
this would typically be called `iterator`, no?
```
for (auto iterator = containerizers.begin(); iterator != containerizers.end();
++iterator) {
Containerizer* containerizer = *iterator;
}
```
Why do something differently when auto is used?
If the iterator was being de-referenced then `containerizer` makes sense:
```
Containerizer* containerizer = *(containerizers.begin());
```
- Ben
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/#review54173
---
On Sept. 22, 2014, 1:10 p.m., Alexander Rukletsov wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/
---
(Updated Sept. 22, 2014, 1:10 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. The discussion about using
namespace aliases took place in [the other
review](https://reviews.apache.org/r/25434/#comment91754). The majority
agreed not to introduce them in code.
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 25549: Basic filesystem isolator for Linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/#review54179 --- Patch looks great! Reviews applied: [25655, 24177, 25549] All tests passed. - Mesos ReviewBot On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/ --- (Updated Sept. 22, 2014, 6:45 p.m.) Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. Bugs: MESOS-1586 https://issues.apache.org/jira/browse/MESOS-1586 Repository: mesos-git Description --- Does not report usage or enforce quota but can create 'private' directories for each container which mask parts of the shared host filesystem. This review replaces https://reviews.apache.org/r/24178/ because of some file renaming. I addressed all comments from earlier reviews. Diffs - include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 Diff: https://reviews.apache.org/r/25549/diff/ Testing --- make check # added a test Thanks, Ian Downes
Re: Review Request 25549: Basic filesystem isolator for Linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/#review54181 --- src/slave/slave.cpp https://reviews.apache.org/r/25549/#comment94177 Thanks for adding this, I was thinking about more about the TaskInfo's container info instead of the CommandExecutor executor info since I'm not sure how the command executor is going to use it yet. I don't want to block your rb though, I can add it in another rb myself :) - Timothy Chen On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/ --- (Updated Sept. 22, 2014, 6:45 p.m.) Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. Bugs: MESOS-1586 https://issues.apache.org/jira/browse/MESOS-1586 Repository: mesos-git Description --- Does not report usage or enforce quota but can create 'private' directories for each container which mask parts of the shared host filesystem. This review replaces https://reviews.apache.org/r/24178/ because of some file renaming. I addressed all comments from earlier reviews. Diffs - include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 Diff: https://reviews.apache.org/r/25549/diff/ Testing --- make check # added a test Thanks, Ian Downes
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 22, 2014, 8:35 p.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Changes --- Review comments. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs (updated) - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25858: Allowed co-mounted cgroup subsystems to enable Mesos on machines with systemd.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/#review54187 --- Ship it! Ship It! - Timothy Chen On Sept. 22, 2014, 8:35 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25858/ --- (Updated Sept. 22, 2014, 8:35 p.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy St. Clair, and Vinod Kone. Bugs: MESOS-1195 https://issues.apache.org/jira/browse/MESOS-1195 Repository: mesos-git Description --- A dynamic version after discussed with Tim. https://reviews.apache.org/r/25695 Did a few consistency fixes as well. Diffs - src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48 src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55 src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e Diff: https://reviews.apache.org/r/25858/diff/ Testing --- make check sudo make check Thanks, Jie Yu
Re: Review Request 25863: Rename stout/os/setns.hpp to namespaces.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25863/#review54193 --- Patch looks great! Reviews applied: [25863] All tests passed. - Mesos ReviewBot On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25863/ --- (Updated Sept. 22, 2014, 6:45 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Also, added os::getns(pid, ns) to get the namespace inode for comparisons between pids' namespaces. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am db9766d70adb9076946cd2b467c55636fe5f7235 3rdparty/libprocess/3rdparty/stout/Makefile.am b6464de53c3873ecd0b62a08ca9aac530043ffb9 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6fa5b741bdd7f089ba93bf6fea43b9f39f8f0edb 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp 5278996f201a4a3d69282c1bd7b0d230d0f6cd39 3rdparty/libprocess/3rdparty/stout/tests/os/namespaces_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp ad8e37aa2f5a29f8b421dde6b7cd5dfe241eabb5 src/slave/containerizer/isolators/network/port_mapping.cpp 9248460caa558e43a2a7d237f6a37d43f0eeacb5 Diff: https://reviews.apache.org/r/25863/diff/ Testing --- Added test to check a clone(NEW_PIDNS) results in a new pid namespace. Thanks, Ian Downes
Re: Review Request 25569: Refactor test environment validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/ --- (Updated Sept. 22, 2014, 9:17 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25569 Diffs (updated) - src/linux/cgroups.cpp 62df4b7645c6ab061a47634058d79ca849caa6b9 src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880 src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652 src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 src/tests/environment.cpp 2274251aaf653d83c2d03ef2186763978067a747 Diff: https://reviews.apache.org/r/25569/diff/ Testing --- make check Thanks, Timothy Chen
Re: Mesos Modules Design
Jumping on a tcon/hangout sounds healthy, but given I'm traveling right now in Europe and timing is difficult I'm going to comment inline here. - callsites need to be modules aware to use the right factory method to instantiate the modular object I don't know how else you'd accomplish making certain components of the system *modular* unless we changed the way we instantiated the components. This one seems fairly self-evident to me, but perhaps there's a trick we could play instead? Are you thinking something LD_PRELOAD-esque here? But works well with C++? - mesos-core owners need to be aware of modules and versioning when they change the abstract API Yes, definitely, anyone hacking in Mesos should be aware that changing the API of something like the Allocator might break all Allocator implementations that exist out there. This is pretty fundamental to module systems and one of the reasons why the design has a flexible, albeit more complicated, verification/versioning component. - module owners need to be aware of the modules API and versioning As they should as well, since it would be nice to give us some flexibility in the future to deal with the mistakes we're bound to make. - customers (end-users) have to be modules aware and set their runtime configuration correctly Mesos will always function without the need for any external modules, as it does today. But yes, if an operator wants to use a module than they will definitely need to update the configuration to do so. The example I'm thinking about here is LoadModule from Apache HTTPD. - errors in versioning or modules will only be caught at runtime This is an artifact of the design constraints. In fact, perhaps it wasn't highlighted explicitly in the design document, but we'd definitely like to support modules in binary format, like an RPM (as you can do with Apache HTTPD modules as well). That implies that a module developer might be building their module independently of an operator building Mesos. Moreover, it implies that if there is anything we definitely want to make rock solid in our design it's what happens when such a module is launched with a running Mesos cluster, hence the design's focus on verification and versioning. Perhaps the design is too complicated and we should settle on something simpler, like forcing every build of a module to be associated with a single Mesos version, but regardless, an operator might still make a mistake when configuring Mesos and we must do the verification at runtime. My alternative proposal requires the following: Sorry if I'm just out of the loop here, but did you share a proposal with the mailing list or on JIRA that I missed? I'm having a hard time following your statements about your proposal without that context. That would definitely help facilitate a discussion via tcon/hangout as well.
Re: Review Request 25862: Improve shared filesystem isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25862/#review54198 --- Bad patch! Reviews applied: [25861] Failed command: git apply --index 25861.patch Error: error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:455 error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply - Mesos ReviewBot On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25862/ --- (Updated Sept. 22, 2014, 6:45 p.m.) Review request for mesos and Vinod Kone. Repository: mesos-git Description --- * Ensure SharedFilesystemIsolator is the first isolator in prepare (and last in cleanup). * Remounts /proc and /sys for the container. * Remove /proc and /sys remounts from PortMappingIsolator. * Make the SharedFilesystemIsolator a dependency for the PortMappingIsolator. Diffs - src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION src/slave/containerizer/isolators/network/port_mapping.cpp 9248460caa558e43a2a7d237f6a37d43f0eeacb5 src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c Diff: https://reviews.apache.org/r/25862/diff/ Testing --- make check Thanks, Ian Downes
Re: Mesos Modules Design
On Mon, Sep 22, 2014 at 2:36 PM, Benjamin Hindman b...@eecs.berkeley.edu wrote: Jumping on a tcon/hangout sounds healthy, but given I'm traveling right now in Europe and timing is difficult I'm going to comment inline here. - callsites need to be modules aware to use the right factory method to instantiate the modular object I don't know how else you'd accomplish making certain components of the system *modular* unless we changed the way we instantiated the components. This one seems fairly self-evident to me, but perhaps there's a trick we could play instead? Are you thinking something LD_PRELOAD-esque here? But works well with C++? I'm thinking that you'd instantiate a module using the same class name but the link step would take care of making sure the modular implementation is available. - mesos-core owners need to be aware of modules and versioning when they change the abstract API Yes, definitely, anyone hacking in Mesos should be aware that changing the API of something like the Allocator might break all Allocator implementations that exist out there. This is pretty fundamental to module systems and one of the reasons why the design has a flexible, albeit more complicated, verification/versioning component. - module owners need to be aware of the modules API and versioning As they should as well, since it would be nice to give us some flexibility in the future to deal with the mistakes we're bound to make. - customers (end-users) have to be modules aware and set their runtime configuration correctly Mesos will always function without the need for any external modules, as it does today. But yes, if an operator wants to use a module than they will definitely need to update the configuration to do so. The example I'm thinking about here is LoadModule from Apache HTTPD. - errors in versioning or modules will only be caught at runtime This is an artifact of the design constraints. In fact, perhaps it wasn't highlighted explicitly in the design document, but we'd definitely like to support modules in binary format, like an RPM (as you can do with Apache HTTPD modules as well). That implies that a module developer might be building their module independently of an operator building Mesos. Moreover, it implies that if there is anything we definitely want to make rock solid in our design it's what happens when such a module is launched with a running Mesos cluster, hence the design's focus on verification and versioning. Perhaps the design is too complicated and we should settle on something simpler, like forcing every build of a module to be associated with a single Mesos version, but regardless, an operator might still make a mistake when configuring Mesos and we must do the verification at runtime. Unless you take the configuration of modules out of runtime and into compile or link time. My alternative proposal requires the following: Sorry if I'm just out of the loop here, but did you share a proposal with the mailing list or on JIRA that I missed? I'm having a hard time following your statements about your proposal without that context. That would definitely help facilitate a discussion via tcon/hangout as well. I've made this proposal on JIRA and on this mailing list, but for simplicity here it is again: - create abstract classes to define interfaces to objects that should be modular - build modules as static libraries that can be assembled at link time to create custom Mesos builds That's it. :) -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 25569: Refactor test environment validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/#review54201 --- Did you look at the diff when you posted this review? Looks like you needed to rebase against master. - Ben Mahler On Sept. 22, 2014, 9:17 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/ --- (Updated Sept. 22, 2014, 9:17 p.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25569 Diffs - src/linux/cgroups.cpp 62df4b7645c6ab061a47634058d79ca849caa6b9 src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880 src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652 src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 src/tests/environment.cpp 2274251aaf653d83c2d03ef2186763978067a747 Diff: https://reviews.apache.org/r/25569/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/#review54203 --- Bad patch! Reviews applied: [25865] Failed command: git apply --index 25865.patch Error: error: patch failed: src/Makefile.am:339 error: src/Makefile.am: patch does not apply error: patch failed: src/slave/containerizer/linux_launcher.cpp:95 error: src/slave/containerizer/linux_launcher.cpp: patch does not apply error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:40 error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply - Mesos ReviewBot On Sept. 22, 2014, 6:46 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/ --- (Updated Sept. 22, 2014, 6:46 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace. Diffs - src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION src/slave/containerizer/linux_launcher.cpp d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 Diff: https://reviews.apache.org/r/25865/diff/ Testing --- Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc). Thanks, Ian Downes
Re: Review Request 25866: Updated the semantics of disconnected/deactivated semantics in master.
On Sept. 21, 2014, 9:20 a.m., Adam B wrote: src/master/master.hpp, lines 947-950 https://reviews.apache.org/r/25866/diff/2/?file=699118#file699118line947 Comment here should be for the Slave. 'active' is set to false if resources from this slave should not be offered. This happens when the slave is disconnected or the master receives a DeactivateSlaveMessage. Do we have a DeactivateSlaveMessage yet? Or does that come with the HTTP endpoint? oops. good catch. fixed. we don't have a DeactivateSlaveMessage yet. not sure when it's going to be introduced. On Sept. 21, 2014, 9:20 a.m., Adam B wrote: src/master/master.cpp, lines 3061-3062 https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line3061 Why are these CHECKs? How should the master respond if it does receive such a message from a deactivated slave? Should we perhaps be sending a Shutdown[Slave]Message, or some sort of DeactivateSlaveMessage? This is a CHECK because it shouldn't happen with the current design (slave is deactivated only when disconnected). When these semantics change (e.g., introduction of DeactivateSlaveMessage) we'll have to change the CHECK. On Sept. 21, 2014, 9:20 a.m., Adam B wrote: src/master/master.cpp, line 1552 https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line1552 Can you explain why this TODO is no longer needed? allocator-frameworkActivated calls allocator-allocate, which will sort roles/frameworks and make initial offers based on a stale notion of the newly reactivated framework's outstanding offers. If the resources were recovered first, the allocator would make fairer offers when the framework is first reactivated. It was a bad TODO (on my part) because it doesn't tell me why I didn't fix it in the first place instead of adding a TODO. Maybe I added the TODO as part of some other cleanup and didn't want to change too much in that review. Anyway, now I addressed the TODO now. On Sept. 21, 2014, 9:20 a.m., Adam B wrote: src/master/master.cpp, line 3988 https://reviews.apache.org/r/25866/diff/2/?file=699119#file699119line3988 Ditto. Isn't this TODO still relevant? fixed. see above. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/#review54094 --- On Sept. 20, 2014, 6:46 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/ --- (Updated Sept. 20, 2014, 6:46 p.m.) Review request for mesos, Adam B and Ben Mahler. Bugs: MESOS-1081 and MESOS-1811 https://issues.apache.org/jira/browse/MESOS-1081 https://issues.apache.org/jira/browse/MESOS-1811 Repository: mesos-git Description --- Made consistent what connected and active frameworks/slaves means. Fixed MESOS-1811 along the way. Diffs - src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/tests/fault_tolerance_tests.cpp 154386044d0247b39d84719d7ff14250682a0695 Diff: https://reviews.apache.org/r/25866/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 25866: Updated semantics of disconnected/deactivated slaves/frameworks in master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/ --- (Updated Sept. 22, 2014, 11:50 p.m.) Review request for mesos, Adam B and Ben Mahler. Changes --- dominic's and adam's comments. PTAL. Summary (updated) - Updated semantics of disconnected/deactivated slaves/frameworks in master. Bugs: MESOS-1081 and MESOS-1811 https://issues.apache.org/jira/browse/MESOS-1081 https://issues.apache.org/jira/browse/MESOS-1811 Repository: mesos-git Description --- Made consistent what connected and active frameworks/slaves means. Fixed MESOS-1811 along the way. Diffs (updated) - src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/tests/fault_tolerance_tests.cpp 154386044d0247b39d84719d7ff14250682a0695 src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 Diff: https://reviews.apache.org/r/25866/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 25866: Updated the semantics of disconnected/deactivated semantics in master.
On Sept. 20, 2014, 2:26 p.m., Dominic Hamon wrote: src/master/master.cpp, line 4649 https://reviews.apache.org/r/25866/diff/1/?file=698461#file698461line4649 consider adding gauges for connected/disconnected slaves done. added for frameworks too. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/#review54078 --- On Sept. 20, 2014, 6:46 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/ --- (Updated Sept. 20, 2014, 6:46 p.m.) Review request for mesos, Adam B and Ben Mahler. Bugs: MESOS-1081 and MESOS-1811 https://issues.apache.org/jira/browse/MESOS-1081 https://issues.apache.org/jira/browse/MESOS-1811 Repository: mesos-git Description --- Made consistent what connected and active frameworks/slaves means. Fixed MESOS-1811 along the way. Diffs - src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/tests/fault_tolerance_tests.cpp 154386044d0247b39d84719d7ff14250682a0695 Diff: https://reviews.apache.org/r/25866/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 25867: Updated ping message to embed the slave registered status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25867/ --- (Updated Sept. 22, 2014, 11:51 p.m.) Review request for mesos and Ben Mahler. Changes --- rebased. Bugs: MESOS-1668 https://issues.apache.org/jira/browse/MESOS-1668 Repository: mesos-git Description --- Embeded slave registration status in ping message to solicit slave re-registration during one way master -- slave partition. Diffs (updated) - src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/messages/messages.proto 7cb3ce651997c04ef1ef95539098ed2a99270b11 src/slave/slave.hpp 4f3df5c49a8cf72fc7153158c9eb045196b6cf13 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e src/tests/partition_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25867/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 25864: Add 'FutureNothing cgroups::empty()'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/#review54211 --- Patch looks great! Reviews applied: [25864] All tests passed. - Mesos ReviewBot On Sept. 22, 2014, 6:46 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25864/ --- (Updated Sept. 22, 2014, 6:46 p.m.) Review request for mesos, Jie Yu and Vinod Kone. Repository: mesos-git Description --- Polls cgroups.procs until no processes in the cgroup. Poll interval and timeout can be specified. Diffs - src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf src/linux/cgroups.cpp 5093b4ca1ac17238234d96613b7f4ceab4373c48 Diff: https://reviews.apache.org/r/25864/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 25848: Introducing mesos modules.
On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/module/manager.cpp, lines 43-44 https://reviews.apache.org/r/25848/diff/1/?file=697029#file697029line43 Ben, we did the module manager as a singleton. I know it is a uncommon pattern in Mesos in general. Do you have any thoughts on this? We will need to take care of thread-safety with whatever state we maintain in the module manager. Ben, any thoughts on this one? - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54043 --- On Sept. 22, 2014, 8:22 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 8:22 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 8:22 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Changes --- Added comments; use shared_ptr; added lock; and addressed some more of Nik's comments. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs (updated) - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25848: Introducing mesos modules.
On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/examples/test_module.cpp, line 38 https://reviews.apache.org/r/25848/diff/1/?file=697027#file697027line38 You could also throw in a comment here on what function declaration that gets generated i.e. exported symbol name and return type. Done! On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/module/manager.cpp, lines 189-215 https://reviews.apache.org/r/25848/diff/1/?file=697029#file697029line189 This block is very dense - mind spreading it out a little bit? Added a TODO for Bernd. On Sept. 19, 2014, 7:39 p.m., Niklas Nielsen wrote: src/tests/module.hpp, line 29 https://reviews.apache.org/r/25848/diff/1/?file=697032#file697032line29 And perhaps highlight the virtual destructor's importance (in order to clean up the object in the library's context). People are probably going to use this as a template for writing new modules :-) Done. - Kapil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review54043 --- On Sept. 22, 2014, 8:22 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 22, 2014, 8:22 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/#review54217 --- Bad patch! Reviews applied: [25911] Failed command: ./support/mesos-style.py Error: Checking 507 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo src/slave/status_update_manager.cpp:189: Lines should be = 80 characters long [whitespace/line_length] [2] src/slave/status_update_manager.hpp:299: Redundant blank line at the start of a code block should be deleted. [whitespace/blank_line] [2] Total errors found: 2 - Mesos ReviewBot On Sept. 22, 2014, 10:30 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- (Updated Sept. 22, 2014, 10:30 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 25866: Updated semantics of disconnected/deactivated slaves/frameworks in master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/#review54215 --- Ship it! Great job. src/master/master.cpp https://reviews.apache.org/r/25866/#comment94218 Only need to set connected=true once. - Adam B On Sept. 22, 2014, 4:50 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25866/ --- (Updated Sept. 22, 2014, 4:50 p.m.) Review request for mesos, Adam B and Ben Mahler. Bugs: MESOS-1081 and MESOS-1811 https://issues.apache.org/jira/browse/MESOS-1081 https://issues.apache.org/jira/browse/MESOS-1811 Repository: mesos-git Description --- Made consistent what connected and active frameworks/slaves means. Fixed MESOS-1811 along the way. Diffs - src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/tests/fault_tolerance_tests.cpp 154386044d0247b39d84719d7ff14250682a0695 src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 Diff: https://reviews.apache.org/r/25866/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 25911: Changed master to free up resources for completed tasks when framework is disconnected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/#review54221 --- src/master/master.cpp https://reviews.apache.org/r/25911/#comment94221 Seems like resources recovered is only used internally for the master, any reason why introducing a new protobuf field instead of just storing it locally? - Timothy Chen On Sept. 22, 2014, 10:30 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25911/ --- (Updated Sept. 22, 2014, 10:30 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1817 https://issues.apache.org/jira/browse/MESOS-1817 Repository: mesos-git Description --- We have run into a problem that cause tasks which completes, when a framework is disconnected and has a fail-over time, to remain in a running state even though the tasks actually finishes. This hogs the cluster and gives users a inconsistent view of the cluster state. The problem turn out to be an issue with the ack-cycle of status updates: If the framework disconnects (with a failover timeout set), the status update manage on the slaves will keep trying to send the front of status update stream to the master (which in turn forwards it to the framework). If the first status update after the disconnect is terminal, things work out fine; the master picks the terminal state up, removes the task and release the resources. If, on the other hand, one non-terminal status is in the stream. The master will never know that the task finished (or failed) before the framework reconnects. As a first pass, this patch makes the status update manager inform the master if a terminal state was found in the pending stream of a task. If so, the master will recover the resources but will still wait the updates to arrive before updating the task state and statuses. Diffs - src/master/master.hpp f5d74ae src/master/master.cpp e5d30e9 src/messages/messages.proto 7cb3ce6 src/slave/status_update_manager.hpp 24e3882 src/slave/status_update_manager.cpp 5d5cf23 src/tests/fault_tolerance_tests.cpp 1543860 Diff: https://reviews.apache.org/r/25911/diff/ Testing --- Added a new test: FaultToleranceTest.RecoverResourcesDuringSchedulerDisconnect which exercise the new code path. make check Thanks, Niklas Nielsen
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
On Sept. 20, 2014, 7:22 p.m., Ben Mahler wrote: Thanks Joris! Any reason you want this exposed in the public include/ folder of libprocess, as opposed to an internal header inside src/? Don't think we'd want this in the public includes. Niklas Nielsen wrote: It seems to be a bit unconsistent whether things have gone in include/ or not. For example, the socket abstraction is only used by libprocess internally and is in include/. The node class ended up there too (my / our fault) - so if we decide to move the private headers to src/, we need to move that alongside doing a scan for the headers we only use inside libprocess. Joris Van Remoortere wrote: Hi Ben, I am refactoring certain classes from process.cpp into the include directory because my goal is to allow pluggable implementations (modules) of the event-management abstraction. My thought process is that we should have the classes required to implement this abstraction available in the public include dir. Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled. I am in the process of making a branch available as a prototype for MESOS-1330; which will make it more apparent where I am going. What are your thoughts on the rules behind the public include dir, and what requirements we can impose on module implementers? Ben Mahler wrote: For the public include directory, if the abstraction is generally useful (e.g. Node, Socket is less clearly useful but looks ok) then it's fine to include these in the public headers. `ProcessReference` however, is not something that the library user can leverage, and it's dangerous to expose: use of it can lead to ProcessManager::cleanup spinning forever! _My thought process is that we should have the classes required to implement this abstraction available in the public include dir._ Are you interested in applying the same philosophy to the mesos code base? That would grow the public headers substantially. _Since this is an open source project, it's not impossible for us to have some of these class declarations / definitions in the src directory, and the module implementer to be required to build it with the src tree pulled._ Seems like a reasonable thing for module implementers IMHO, otherwise we may have to expose a lot of internals in the public include folder of both libprocess and mesos. Joris Van Remoortere wrote: Thanks for your feedback Ben. I'm fine with requiring module implementers to pull the src; I just did not want to force the issue without a discussion. If everyone else is ok with this then I will keep things in the src dirs. Here is a prototype for the event management refactor I would like to do for MESOS-1330: https://github.com/mesosphere/mesos/tree/abstract-event-manager I will modify this diff to move the process_reference into src as discussed. (I have followed the same pattern in the above prototype). - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54086 --- On Sept. 20, 2014, 12:58 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 20, 2014, 12:58 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 542ae1c 3rdparty/libprocess/include/process/process_reference.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 8adc809 Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 23, 2014, 1:18 a.m.) Review request for mesos and Niklas Nielsen. Changes --- Move ProcessReference to src dir from include dir. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs (updated) - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/src/process.cpp 8adc809 3rdparty/libprocess/src/process_reference.hpp PRE-CREATION Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25867: Updated ping message to embed the slave registered status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25867/#review54240 --- Patch looks great! Reviews applied: [25866, 25867] All tests passed. - Mesos ReviewBot On Sept. 22, 2014, 11:51 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25867/ --- (Updated Sept. 22, 2014, 11:51 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1668 https://issues.apache.org/jira/browse/MESOS-1668 Repository: mesos-git Description --- Embeded slave registration status in ping message to solicit slave re-registration during one way master -- slave partition. Diffs - src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 src/messages/messages.proto 7cb3ce651997c04ef1ef95539098ed2a99270b11 src/slave/slave.hpp 4f3df5c49a8cf72fc7153158c9eb045196b6cf13 src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e src/tests/partition_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25867/diff/ Testing --- make check Thanks, Vinod Kone
Review Request 25922: Explore disk io isolation in cgroups
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25922/ --- Review request for mesos, Adam B and Benjamin Hindman. Repository: mesos-git Description --- Following from: r25105 Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor. Diffs - include/mesos/mesos.proto be45494 include/mesos/resources.hpp 0e37170 src/Makefile.am 9b973e5 src/common/resources.cpp e9a0c85 src/linux/cgroups.hpp abf31df src/linux/cgroups.cpp 5093b4c src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION src/slave/containerizer/mesos/containerizer.cpp 9d08329 src/tests/isolator_tests.cpp c38f876 Diff: https://reviews.apache.org/r/25922/diff/ Testing --- make check sudo bin/mesos-tests.sh --gtest_filter=*BlkIO* support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25789: Variadic strings join
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789/#review54238 --- Ship it! Looks great! A few minor nits and a request for a comment. I'd be willing to commit this unless @benjaminhindman has any objections. 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94247 Nit: You only need single-spacing between a namespace opening and the first code inside it. 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94248 Also don't need double-spacing here (but do below it, I believe) 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp https://reviews.apache.org/r/25789/#comment94249 Maybe you could comment why there are two THeads here (for disambiguating from the Iterable version below)? So the next time somebody looks at this they'll know why/how not to change it. 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp https://reviews.apache.org/r/25789/#comment94250 s/narly/gnarly/? - Adam B On Sept. 22, 2014, 10:30 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25789/ --- (Updated Sept. 22, 2014, 10:30 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Add Variadic strings join. There is a second version of the variadic join which takes a reference to a stringstream as a parameter. This is handy when strings::join is just a part of a larger string manipulation. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25789/diff/ Testing --- Ran make check for stout. Added test cases for join as these were missing. Thanks, Joris Van Remoortere
Re: Review Request 25848: Introducing mesos modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Sept. 23, 2014, 12:05 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and Timothy St. Clair. Changes --- Fixed minor typo. Bugs: MESOS-1384 https://issues.apache.org/jira/browse/MESOS-1384 Repository: mesos-git Description --- Adding a first class primitive, abstraction and process for dynamic library writing and loading can make it easier to extend inner workings of Mesos. Making it possible to have dynamic loadable resource allocators, isolators, containerizes, authenticators and much more. Diffs (updated) - include/mesos/module.hpp PRE-CREATION src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 src/examples/test_module.hpp PRE-CREATION src/examples/test_module_impl.cpp PRE-CREATION src/module/manager.hpp PRE-CREATION src/module/manager.cpp PRE-CREATION src/tests/module_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25848/diff/ Testing --- Ran make check with added tests for verifying library/module loading and simple version check. Thanks, Kapil Arya
Re: Review Request 25868: Refactor Libprocess: class ProcessReference
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/#review54245 --- Patch looks great! Reviews applied: [25868] All tests passed. - Mesos ReviewBot On Sept. 23, 2014, 1:18 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25868/ --- (Updated Sept. 23, 2014, 1:18 a.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class ProcessReference out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/Makefile.am edbe54b 3rdparty/libprocess/src/process.cpp 8adc809 3rdparty/libprocess/src/process_reference.hpp PRE-CREATION Diff: https://reviews.apache.org/r/25868/diff/ Testing --- make check in 3rdparty/libprocess support/mesos-style.py Thanks, Joris Van Remoortere
Re: Review Request 25569: Refactor test environment validations
On Sept. 22, 2014, 10:52 p.m., Ben Mahler wrote: Did you look at the diff when you posted this review? Looks like you needed to rebase against master. Sorry didn't really look at it and you're right it's not rebased! - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/#review54201 --- On Sept. 23, 2014, 5:28 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/ --- (Updated Sept. 23, 2014, 5:28 a.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25569 Diffs - src/tests/environment.cpp 2274251aaf653d83c2d03ef2186763978067a747 Diff: https://reviews.apache.org/r/25569/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 25569: Refactor test environment validations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/ --- (Updated Sept. 23, 2014, 5:28 a.m.) Review request for mesos and Ben Mahler. Repository: mesos-git Description --- Review: https://reviews.apache.org/r/25569 Diffs (updated) - src/tests/environment.cpp 2274251aaf653d83c2d03ef2186763978067a747 Diff: https://reviews.apache.org/r/25569/diff/ Testing --- make check Thanks, Timothy Chen
[RESULT][VOTE] Release Apache Mesos 0.20.1 (rc3)
Hi all, The vote for Mesos 0.20.1 (rc3) has passed with the following votes. +1 (Binding) -- *** Vinod Kone *** Till Toenshoff *** Niklas Nielsen *** Jie Yu +1 (Non-binding) -- *** Tim Chen *** Tom Arnfeld There were no 0 or -1 votes. Please find the release at: https://dist.apache.org/repos/dist/release/mesos/0.20.1 It is recommended to use a mirror to download the release: http://www.apache.org/dyn/closer.cgi The CHANGELOG for the release is available at: https://git-wip-us.apache.org/repos/asf?p=mesos.git;a=blob_plain;f=CHANGELOG;hb=0.20.1 The mesos-0.20.1.jar has been released to: https://repository.apache.org The website (http://mesos.apache.org) will be updated shortly to reflect this release. Thanks, -Adam-
