mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Lai <ja...@jasonlai.net>
Subject Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem
Date Wed, 21 Dec 2016 22:24:04 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message