mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul Brett" <pau...@twopensource.com>
Subject Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)
Date Thu, 21 May 2015 18:05:08 GMT


> On April 25, 2015, 1:03 a.m., Chi Zhang wrote:
> > include/mesos/mesos.proto, lines 432-438
> > <https://reviews.apache.org/r/32660/diff/3/?file=941248#file941248line432>
> >
> >     maybe make it clear that delayed maps to capping; dropped maps to NIC capacity
exceeded?

I'm think the interpretation depends on which class and interface the statistics is being
reported against.  Jie asked that I expose these statistics for each of the tc disciplines
we put in place, so we will get these same stats reported for each.


> On April 25, 2015, 1:03 a.m., Chi Zhang wrote:
> > include/mesos/mesos.proto, line 451
> > <https://reviews.apache.org/r/32660/diff/3/?file=941248#file941248line451>
> >
> >     required makes it hard to update a protobuf:
> >     
> >     https://developers.google.com/protocol-buffers/docs/proto#updating

I am making everything except the id as optional for exactly this reason, however I believe
it is the normal usage pattern to make the id required.


> On April 25, 2015, 1:03 a.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 620
> > <https://reviews.apache.org/r/32660/diff/3/?file=941250#file941250line620>
> >
> >     is this c++11?

The foreach/auto is the new c++11 way of iterating through a collection.


> On April 25, 2015, 1:03 a.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 2527
> > <https://reviews.apache.org/r/32660/diff/3/?file=941250#file941250line2527>
> >
> >     mind explaining why you call them limiter and fairshare?

Names are an attempt to abstract the statistics from the implementation in case we need to
change the implementatoin in the future.


> On April 25, 2015, 1:03 a.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 2363-2381
> > <https://reviews.apache.org/r/32660/diff/3/?file=941250#file941250line2363>
> >
> >     personally i think just setting them directly is the easist to understand.
> >     
> >     see comment below.

The use of the lambda separates out the logic from the binding of name to setter.  I'll clean
it up a little more and then re-review.


- Paul


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


On April 24, 2015, 7:42 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32660/
> -----------------------------------------------------------
> 
> (Updated April 24, 2015, 7:42 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
> 
> 
> Bugs: mesos-2332
>     https://issues.apache.org/jira/browse/mesos-2332
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Report network isolator statistics on a per container basis (MESOS-2332)
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 466cd82e69665af217d61392b739b9bba16e1e13

>   src/slave/containerizer/isolators/network/port_mapping.cpp ccdc44f465f204f674b859c429ba1a6ada51cd88

>   src/slave/containerizer/mesos/containerizer.hpp fb334e53bc8d73bced402095daddab5e06dc2dd1

>   src/slave/containerizer/mesos/containerizer.cpp ea3b499258b6be4a643d246159a17a0eaeedf904

> 
> Diff: https://reviews.apache.org/r/32660/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


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