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 20:19:47 GMT


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


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