----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review71081 -----------------------------------------------------------
src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment116683> Where are the possible values for 'level' documented? Where's the validation (apart from failing to register the eventfd)? It should be very clear what the three levels are and how to create listeners for them. src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment116671> Make clear that this is the accumulation since monitoring started, i.e., it starts from 0. Also (from below), document that monitoring stops if there's any error. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116672> Any reason why you're not waiting for the process to terminate? src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116676> just name this eventfd to be consistent with elsewhere. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116685> where are internal::registerNotifier and internal::unregisterNotifier defined? Can you please include them in this review or link as a 'depends on' review. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116678> unused argument fd, you just use the member variable eventfd. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116679> listen() src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment116680> Document that on a single error you stop listening. How about have the counter as a Result<uint64_t> so you don't need the additional Option<Error>? - Ian Downes On Feb. 4, 2015, 11:50 a.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30545/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 11:50 a.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: MESOS-2136 > https://issues.apache.org/jira/browse/MESOS-2136 > > > Repository: mesos > > > Description > ------- > > cgroups: added support to listen on memory pressures. > > > Diffs > ----- > > src/linux/cgroups.hpp abf31df > src/linux/cgroups.cpp 0b136e1 > > Diff: https://reviews.apache.org/r/30545/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
