Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 14, 2015, 5:05 p.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 14, 2015, 2:53 a.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 14, 2015, 12:38 a.m.) Review request for mesos, Ian Downes, Jie Y

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review91526 --- Ship it! Ship It! src/linux/cgroups.cpp (line 2022)

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 10, 2015, 10:46 p.m.) Review request for mesos, Ian Downes, Jie Y

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review91343 --- src/linux/cgroups.hpp (line 438)

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 10, 2015, 8:47 p.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 10, 2015, 1:06 a.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 9, 2015, 7:36 p.m., Ben Mahler wrote: > > src/linux/cgroups.hpp, lines 438-447 > > > > > > What is the plan for introducing javadoc comments? cgroups.hpp seems > > like a reasonable candidate, but we shoul

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review91168 --- src/linux/cgroups.hpp (lines 438 - 447)

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review91129 --- src/tests/cgroups_tests.cpp (line 1191)

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review91119 --- src/linux/cgroups.hpp (line 445)

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.hpp, lines 443-472 > > > > > > Thanks! > > > > (1) Do you mind updating my TODO on cgroups::stat() to reflect that > > cpuacct::stat is i

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90636 --- Patch looks great! Reviews applied: [36106] All tests passed. - M

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 7, 2015, 12:26 a.m.) Review request for mesos, Ian Downes, Jie Yu

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.hpp, lines 443-472 > > > > > > Thanks! > > > > (1) Do you mind updating my TODO on cgroups::stat() to reflect that > > cpuacct::stat is i

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90581 --- Hey Jojy, great work so far! Thanks for writing up the JIRA first ;-

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2014 > > > > > > I don't think we usually define a inline function like this anywhere > > else, so curious to see what others think. >

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 6, 2015, 6:20 p.m.) Review request for mesos, Ian Downes, Jie Yu,

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90407 --- Patch looks great! Reviews applied: [36106] All tests passed. - M

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 5, 2015, 6:30 p.m.) Review request for mesos, Ian Downes, Jie Yu,

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.cpp, lines 2014-2027 > > > > > > Any reason you're not just re-using cgroups::stat here? I'd suggest > > just calling cgroups::stat for now, shou

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90387 --- Patch looks great! Reviews applied: [36106] All tests passed. - M

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.cpp, lines 2014-2027 > > > > > > Any reason you're not just re-using cgroups::stat here? I'd suggest > > just calling cgroups::stat for now, shou

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.cpp, lines 2014-2027 > > > > > > Any reason you're not just re-using cgroups::stat here? I'd suggest > > just calling cgroups::stat for now, shou

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.cpp, lines 2014-2027 > > > > > > Any reason you're not just re-using cgroups::stat here? I'd suggest > > just calling cgroups::stat for now, shou

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.cpp, lines 2014-2027 > > > > > > Any reason you're not just re-using cgroups::stat here? I'd suggest > > just calling cgroups::stat for now, shou

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 2, 2015, 11:32 p.m., Ben Mahler wrote: > > src/linux/cgroups.hpp, lines 443-472 > > > > > > Thanks! > > > > (1) Do you mind updating my TODO on cgroups::stat() to reflect that > > cpuacct::stat is i

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90288 --- Are you planning to also update cpushare.cpp? src/linux/cgroups.hp

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2014 > > > > > > I don't think we usually define a inline function like this anywhere > > else, so curious to see what others think. >

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90268 --- Ship it! I'm not entirely sure the Doxygen comments fully comply wi

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 2, 2015, 6:01 p.m.) Review request for mesos, Ian Downes, Joris V

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:54 p.m., Marco Massenzio wrote: > > src/linux/cgroups.cpp, line 2002 > > > > > > and, in any event, the variables should have a meaningful name - should > > these be `userTime` or something? > >

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/tests/cgroups_tests.cpp, line 1192 > > > > > > Add using to the top of the tests > > Jojy Varghese wrote: > Since this is an alias (typedef) and scoped fo

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2060 > > > > > > Why add trailing underscore? > > Jojy Varghese wrote: > As a member variable(is accepted according to mesos codin

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90174 --- Patch looks great! Reviews applied: [36106] All tests passed. - M

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2060 > > > > > > Why add trailing underscore? > > Jojy Varghese wrote: > As a member variable(is accepted according to mesos codin

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2014 > > > > > > I don't think we usually define a inline function like this anywhere > > else, so curious to see what others think. >

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:54 p.m., Marco Massenzio wrote: > > src/linux/cgroups.cpp, line 2002 > > > > > > and, in any event, the variables should have a meaningful name - should > > these be `userTime` or something? > >

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2060 > > > > > > Why add trailing underscore? > > Jojy Varghese wrote: > As a member variable(is accepted according to mesos codin

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2014 > > > > > > I don't think we usually define a inline function like this anywhere > > else, so curious to see what others think. >

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90140 --- src/linux/cgroups.hpp (lines 438 - 440)

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/tests/cgroups_tests.cpp, line 1192 > > > > > > Add using to the top of the tests Since this is an alias (typedef) and scoped for the function. - Jojy

Re: Review Request 36106: cgroups: added cpuacct subsystem

> On July 1, 2015, 9:46 p.m., Timothy Chen wrote: > > src/linux/cgroups.cpp, line 2014 > > > > > > I don't think we usually define a inline function like this anywhere > > else, so curious to see what others think. >

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90139 --- src/linux/cgroups.cpp (line 2002)

Re: Review Request 36106: cgroups: added cpuacct subsystem

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 1, 2015, 9:38 p.m.) Review request for mesos, Ian Downes, Joris V