Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated March 27, 2015, 3:18 a.m.) Review request for mesos, Dominic Hamon, Ia

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On March 25, 2015, 10:57 p.m., Ben Mahler wrote: > > src/linux/cgroups.hpp, lines 547-575 > > > > > > One initial confusion for me was that "Listener" seemed to suggest that > > it "listens" and tells us when an ev

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review77819 --- Looks good, just a minor API suggestion below. src/linux/cgroups.h

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review77631 --- Ship it! Ship It! - Ian Downes On March 11, 2015, 3:30 p.m., Chi

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated March 11, 2015, 10:30 p.m.) Review request for mesos, Dominic Hamon, I

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 23, 2015, 8:06 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 23, 2015, 5:15 p.m., Dominic Hamon wrote: > > src/linux/cgroups.cpp, line 2365 > > > > > > maybe CHECK_NOTNULL(process.get()) in case someone is trying to call > > counter during Listener destruction? Proc

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 23, 2015, 5:28 p.m., Ian Downes wrote: > > src/linux/cgroups.cpp, line 2276 > > > > > > indentation kept the old format :) - Chi --- This is an au

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73605 --- Ship it! This is great Chi! Giving you a shipit and starting to loo

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73587 --- src/linux/cgroups.cpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73585 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 23, 2015, 4:43 a.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 20, 2015, 9:56 p.m., Dominic Hamon wrote: > > src/linux/cgroups.cpp, line 2343 > > > > > > why does the Listener not create the ListenerProcess in its constructor > > and therefore own the lifetime of it?

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 22, 2015, 5:13 a.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

- Chi --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73359 --- On Feb. 22, 2015, 5:13 a.m., Chi Zhang wrote: > >

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 21, 2015, 1:27 a.m., Jie Yu wrote: > > src/linux/cgroups.cpp, lines 2279-2280 > > > > > > I think here should be able to do: > > ``` > > : coutner_(0), > > error(None()), > > proces

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73359 --- The code looks great! Given that the tests are quite large. Can you

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73320 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 20, 2015, 9:37 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review72971 --- First pass, haven't looked at the tests yet. The main issue is that

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 17, 2015, 8:12 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review72760 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 13, 2015, 6:08 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 12, 2015, 9:06 p.m., Dominic Hamon wrote: > > src/linux/cgroups.hpp, line 344 > > > > > > if the caller is responsible for the lifetime of the process, this > > should be a unique_ptr (or Owned) > > Chi Zha

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 12, 2015, 9:06 p.m., Dominic Hamon wrote: > > src/linux/cgroups.cpp, line 1333 > > > > > > i don't understand why the first reading is special in this case. replyed the several comments above altogether bel

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review72242 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 3, 2015, 5:42 p.m., Dominic Hamon wrote: > > src/tests/cgroups_tests.cpp, line 691 > > > > > > on oom? you mean a critical memory pressure event? meant to oom with oom-killer disabled; the test has been disc

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 3, 2015, 6:42 p.m., Ian Downes wrote: > > src/tests/cgroups_tests.cpp, lines 621-622 > > > > > > We should be able to test each of the levels separately, i.e., we must > > understand these events so we can p

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 3, 2015, 9:16 a.m., Timothy Chen wrote: > > src/tests/cgroups_tests.cpp, line 652 > > > > > > Shouldn't you verify that discard actually terminates the process? The test was rewritten. - Chi

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 4, 2015, 11:47 p.m., Dominic Hamon wrote: > > src/linux/cgroups.hpp, line 602 > > > > > > why is there a trailing underscore? there is a function counter(); c++ doesn't allow a member function and a field t

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 5, 2015, 12:50 a.m., Jie Yu wrote: > > Per our dicussion, could you expose and reuse the existing EventListener? > > Here are the thoughts: > > > > 1) s/EventListener/EventListenerProcess > > 2) expose EventListner (the wrapper for EventListenerProcess) in the header > > file > > 3)

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 12, 2015, 8:08 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review71104 --- Per our dicussion, could you expose and reuse the existing EventList

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review71081 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review71078 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- (Updated Feb. 4, 2015, 7:50 p.m.) Review request for mesos, Dominic Hamon, Ian

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- 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.

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review70772 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

> On Feb. 3, 2015, 9:06 a.m., Nikita Vetoshkin wrote: > > src/linux/cgroups.cpp, line 2345 > > > > > > Wouldn't `new` throw instead of returning null? I believe it will throw bad_alloc, the check is unnecessary. -

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review70719 --- src/linux/cgroups.hpp

Re: Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review70720 --- src/linux/cgroups.cpp

Review Request 30545: cgroups: added support to listen on memory pressures.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/ --- Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. Bugs: MESOS-21