----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30546/#review77084 -----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/mem.cpp <https://reviews.apache.org/r/30546/#comment124928> Non-POD global variables can cause issues during teardown in a multi-threaded enviornment. So the google style guide bans it. You can use a static local function instead (no need to declare it in the header): ``` vector<Level> levels() { return {Level::LOW, Level::MEDIUM, LEVEL::CRITICAL}; } ``` src/slave/containerizer/isolators/cgroups/mem.cpp <https://reviews.apache.org/r/30546/#comment124931> "Test if pressure listening is enabled. ..." src/slave/containerizer/isolators/cgroups/mem.cpp <https://reviews.apache.org/r/30546/#comment124935> We are not supposed to pass 'Owned' pointer around because once it's copyed, the ownership has tansferred and the owned pointers in info->pressureListeners will become invalid. Also, I don't think the chaining here improves the readability (it actually makes the code a little hard to follow). However, I do think using a foreachpair helps simply the code here. So here is my suggestion: ``` list<Level> levels; list<Future<uint64_t>> futures; foreachpair (...) { levels.push_back(level); futures.push_back(listener->counter()); } return await(futures) .then(defer(..., &_usage, containerId, levels, lambda::_1)); ``` - Jie Yu On March 18, 2015, 6:32 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30546/ > ----------------------------------------------------------- > > (Updated March 18, 2015, 6:32 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: MESOS-2136 > https://issues.apache.org/jira/browse/MESOS-2136 > > > Repository: mesos > > > Description > ------- > > MemIsolator: expose memory pressures for containers. > > > Diffs > ----- > > include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 > src/slave/containerizer/isolators/cgroups/mem.hpp > a00f723de61b9bccd76a2948b6d14fde7a993a8d > src/slave/containerizer/isolators/cgroups/mem.cpp > 6299ca4ba01b65daa3d75c64150e2738e32b841e > src/tests/slave_recovery_tests.cpp 53adae0118a26e6d25a9ff20c6374cc8e73275b1 > > Diff: https://reviews.apache.org/r/30546/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
