----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review70786 -----------------------------------------------------------
This is high level review about the interface and architecture only. Subsequent review once these concerns are addressed will be more detailed. src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment116216> This interface is cumbersome - why not have separate listen calls for the level that you want rather than some combination of Option<>s? What is the use-case for only listening to a subset of levels? src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment116213> What are the execution semantics around these callbacks? In what context are they executed? src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116215> What does 'deliver' mean... What exactly is callback doing here? src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116218> Why do we need a manager for these? We could start individual listeners for each level (see previous comment) as needed and have each handle discarding the future to terminate. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/30545/#comment116221> We should be able to test each of the levels separately, i.e., we must understand these events so we can precisely manipulate our memory usage to trigger each of them. I'm not sure what this test is doing, it seems to be trying to only reach the critical level (OOM) by setting the limit to 64 MB and attempting to use 256 MB. - Ian Downes On Feb. 2, 2015, 9:28 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30545/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2015, 9:28 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: MESOS-2136 > https://issues.apache.org/jira/browse/MESOS-2136 > > > Repository: mesos > > > Description > ------- > > cgroups: added support to listen on memory pressures. > > > Diffs > ----- > > src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf > src/linux/cgroups.cpp 0b136e17907123e814e6011523d8e66a4d66cf98 > src/tests/cgroups_tests.cpp 1eea57f217cd51a6cf0a59ca80823251333563c6 > > Diff: https://reviews.apache.org/r/30545/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
