> On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 2188 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2188> > > > > I'd introduce CgroupInfo::Blkio::CFQ CgroupInfo::Blkio::Throttling > > message here. Looks like for the fields below, we should separate them. For > > instance, `read_bps_device` is only for throttling iosched.
I have some reservation on introducing message `CFQ` for both config and stats. CFQ may be good for the stats part, but the configuration-related control files involved only include are fairly straightforward: * `blkio.weight` (also `_device`) * `blkio.leaf_weight` (also `_device`). Do you think we need to introduce an extra level as a field named `cfq` for weights? > On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 2214 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2214> > > > > I am wondering if the following structure is better: > > > > ``` > > message Blkio { > > message CFQ { > > message Statistics { > > optional Device device; > > optional uint64 sectors; > > optional uint64 time; > > repeated Entry io_serviced; > > ... > > } > > > > repeated DeviceWeight weight_devices; > > repeated DeviceWeight leaf_weight_devices; > > } > > > > message Throttling { > > message Statistics { > > ... > > } > > > > repeated DeviceLimit write_bps_devices; > > repeated DeviceLimit write_iops_devices; > > } > > > > message Statistics { > > repeated CFQ::Statistics cfq; > > repeated CFQ::Statistics cfq_recursive; > > } > > > > optional CFQ cfq; > > optional Throttling throttling; > > } > > > > message ResourceStatistics { > > optional Blkio::Statistics blkio; > > } > > ``` > > Jason Lai wrote: > By introducing `CgroupInfo::ResourceStatistics`, can I guess that you are > suggesting that we have a field with this message named `cgroups` within the > global `ResourceStatistics` like: > > ``` > messge ResourceStatistics { > // .... > optional CgroupInfo.ResourceStatistics cgroups = 43; > // .... > } > > message CgroupInfo { > // .... > > message Blkio { > // .... > message Statistics { > // .... > } > // .... > } > > message ResourceStatistics { > optional Blkio.Statistics blkio = 1; > } > > // .... > } > ``` I have some further thoughts on the control fields for `CgroupInfo::Blkio`. I would like to see if it makes sense to you if we put the cgroup's `weight` and `leaf_weight` as direct fields and group the per-device configurations in the same way we do for stats? i.e. Something like: ``` message Blkio message DeviceLimits { required Device device = 1; optional uint32 weight = 2; optional uint32 leaf_weight = 3; optional uint64 read_bps = 4; optional uint64 write_bps = 5; optional uint64 read_iops = 6; optional uint64 write_iops = 7; } message Statistics { // .... } optional uint32 weight = 1; optional uint32 leaf_weight = 2; repeated DeviceLimits device_limits = 3; } ``` - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54693/#review159381 ----------------------------------------------------------- On Dec. 21, 2016, 8:06 p.m., Jason Lai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54693/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2016, 8:06 p.m.) > > > Review request for mesos, Xiaojian Huang, Gilbert Song, haosdent huang, Jie > Yu, Kunal Thakar, and Zhitao Li. > > > Bugs: MESOS-6162 > https://issues.apache.org/jira/browse/MESOS-6162 > > > Repository: mesos > > > Description > ------- > > Add ProtoBuf schema for Blkio cgroups per the kernel Blkio controller doc. > > > Diffs > ----- > > include/mesos/mesos.proto 0017d3d9d46433b391703025f611ce437bbc7ebe > > Diff: https://reviews.apache.org/r/54693/diff/ > > > Testing > ------- > > > Thanks, > > Jason Lai > >
