> On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, lines 2226-2227 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2226> > > > > What are these two?
They are `blkio.throttle.io_serviced` and `blkio.throttle.io_service_bytes`. I'll add comments to them. > On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp, lines > > 45-51 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582393#file1582393line45> > > > > Let's do that in the following patch where you actually get those > > statistics. Sure. I was initially hoping to add code for implementations, but seems like it will make the scope of this diff much bigger. I'll revert these 2 files in the diff. > 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; > > } > > ``` 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; } // .... } ``` > On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 2199 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2199> > > > > Can this be nested inside CgroupInfo::Blkio? Yes, sir. It is done :) > On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 2209 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2209> > > > > s/StatEntry/Entry/ Done. > On Dec. 17, 2016, 1:25 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, lines 2200-2207 > > <https://reviews.apache.org/r/54693/diff/1/?file=1582391#file1582391line2200> > > > > I'd move this to CgroupInfo::Blkio::Operation Done. I have instead moved it to `CgroupInfo::Blkio::Statistics::Operation` as it is stats-specific. - 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 > >
