mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem
Date Sat, 17 Dec 2016 01:25:46 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54693/#review159381
-----------------------------------------------------------




include/mesos/mesos.proto (line 899)
<https://reviews.apache.org/r/54693/#comment230364>

    2 lines apart please.



include/mesos/mesos.proto (line 2188)
<https://reviews.apache.org/r/54693/#comment230551>

    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.



include/mesos/mesos.proto (line 2199)
<https://reviews.apache.org/r/54693/#comment230542>

    Can this be nested inside CgroupInfo::Blkio?



include/mesos/mesos.proto (lines 2200 - 2207)
<https://reviews.apache.org/r/54693/#comment230548>

    I'd move this to CgroupInfo::Blkio::Operation



include/mesos/mesos.proto (line 2209)
<https://reviews.apache.org/r/54693/#comment230557>

    s/StatEntry/Entry/



include/mesos/mesos.proto (line 2214)
<https://reviews.apache.org/r/54693/#comment230555>

    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;
    }
    ```



include/mesos/mesos.proto (lines 2226 - 2227)
<https://reviews.apache.org/r/54693/#comment230554>

    What are these two?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp (lines 45 - 51)
<https://reviews.apache.org/r/54693/#comment230543>

    Let's do that in the following patch where you actually get those statistics.


- Jie Yu


On Dec. 13, 2016, 5:12 a.m., Jason Lai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54693/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2016, 5:12 a.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 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54693/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Lai
> 
>


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