----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59960/#review178013 -----------------------------------------------------------
This patch looks good to me in general. Could we add a simple unit test for these helpers? src/linux/cgroups.hpp Lines 442 (patched) <https://reviews.apache.org/r/59960/#comment251763> Add comments. src/linux/cgroups.hpp Lines 442-458 (patched) <https://reviews.apache.org/r/59960/#comment251766> Is it possible to unify these two struct? ``` struct Entry { Option<Device> device; Option<Operation> operation; uint64_t value; static Try<Entry> parse(const std::string& s); }; ``` src/linux/cgroups.hpp Lines 451 (patched) <https://reviews.apache.org/r/59960/#comment251764> Add comments. src/linux/cgroups.hpp Lines 464-568 (patched) <https://reviews.apache.org/r/59960/#comment251767> Should we have corresponding `namespace` as `throttle`? Considering we have `cfq` and `cfq_recursive` in protobuf. E.g., ``` namespace cfq {} namespace cfq_recursive {} ``` - Gilbert Song On June 9, 2017, 4:24 p.m., Jason Lai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59960/ > ----------------------------------------------------------- > > (Updated June 9, 2017, 4:24 p.m.) > > > Review request for mesos, Eric Chung, Xiaojian Huang, Gilbert Song, haosdent > huang, Jie Yu, and Zhitao Li. > > > Bugs: MESOS-6162 > https://issues.apache.org/jira/browse/MESOS-6162 > > > Repository: mesos > > > Description > ------- > > * Data structure for Blkio entities > * Stats helpers for `blkio.throttle.io*` (generic blkio stats) > * Stats helpers for `blkio.io*` (CFQ related stats) > * Comments from the kernel blkio doc for helper functions > > > Diffs > ----- > > src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 > src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 > > > Diff: https://reviews.apache.org/r/59960/diff/1/ > > > Testing > ------- > > > Thanks, > > Jason Lai > >