Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============0120197088947801986=="
MIME-Version: 1.0
Subject: Re: Review Request 34426: Report per-container metrics for network
 bandwidth throttling
From: "Paul Brett" <paul_b@twopensource.com>
To: "Cong Wang" <cwang@twopensource.com>, "Jie Yu" <yujie.jay@gmail.com>,
 "Ian Downes" <ian.downes@gmail.com>, "Chi Zhang" <chzhcn88@gmail.com>
Cc: "Paul Brett" <paul_b@twopensource.com>, "mesos" <reviews@mesos.apache.org>
Date: Thu, 04 Jun 2015 01:14:33 -0000
Message-ID: <20150604011433.7996.14479@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Paul Brett" <noreply@reviews.apache.org>
References: <20150603223514.7996.52381@reviews.apache.org>
In-Reply-To: <20150603223514.7996.52381@reviews.apache.org>
Reply-To: "Paul Brett" <paul_b@twopensource.com>

--===============0120197088947801986==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On June 3, 2015, 10:35 p.m., Jie Yu wrote:
> > src/linux/routing/queueing/internal.hpp, line 320
> > <https://reviews.apache.org/r/34426/diff/11/?file=977689#file977689line320>
> >
> >     Why <=?

The definition for RTNL_TC_STATS_MAX comes from libnl and looks like this:

enum rtnl_tc_stat {
  RTNL_TC_PACKETS,  /**< Number of packets seen */
  RTNL_TC_BYTES,    /**< Total bytes seen */
  RTNL_TC_RATE_BPS, /**< Current bits/s (rate estimator) */
  RTNL_TC_RATE_PPS, /**< Current packet/s (rate estimator) */
  RTNL_TC_QLEN,   /**< Current queue length */
  RTNL_TC_BACKLOG,  /**< Current backlog length */
  RTNL_TC_DROPS,    /**< Total number of packets dropped */
  RTNL_TC_REQUEUES, /**< Total number of requeues */
  RTNL_TC_OVERLIMITS, /**< Total number of overlimits */
  __RTNL_TC_STATS_MAX,
};
#define RTNL_TC_STATS_MAX (__RTNL_TC_STATS_MAX - 1)

So the range is from 0 to RTNL_TC_STATS_MAX inclusive.


> On June 3, 2015, 10:35 p.m., Jie Yu wrote:
> > src/linux/routing/queueing/statistics.hpp, line 23
> > <https://reviews.apache.org/r/34426/diff/11/?file=977690#file977690line23>
> >
> >     Should we get rid of 'queueing' here because this can be used by filters as well?

It should work on filters but we have no use case and no testing to know if it works.  Lets leave it here until that changes.


- Paul


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


On June 3, 2015, 9:19 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
> 
> (Updated June 3, 2015, 9:19 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2665
>     https://issues.apache.org/jira/browse/MESOS-2665
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Report the network statistics from libnl
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 66030c4b211ea61e97e62c35ec1821e0958f9787 
>   src/linux/routing/queueing/fq_codel.hpp 49c8df626addcfc841132f821b8697b5e6e3b341 
>   src/linux/routing/queueing/fq_codel.cpp 976c6a7a11d39609e6597daeb1a74c93ac62e4bf 
>   src/linux/routing/queueing/ingress.hpp 2b7e1d3d1c02120195b6697320fb46f085a1ef92 
>   src/linux/routing/queueing/ingress.cpp e96f547200dc4ad5b2e33fd1ffbd7fb92b955a46 
>   src/linux/routing/queueing/internal.hpp 3713f6a4d3f9a44b142b8150e85cf911e53e34e9 
>   src/linux/routing/queueing/statistics.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34426/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


--===============0120197088947801986==--