You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Paul Brett <pa...@twopensource.com> on 2015/05/19 20:54:34 UTC
Review Request 34426: Extend the Traffic Control queueing models to
allow dynamically allocated Handles,
extend API to allow searching by handle (or parent).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
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
-------
Extend the Traffic Control queueing models to allow dynamically allocated Handles, extend API to allow searching by handle (or parent).
Diffs
-----
src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
> On June 1, 2015, 8:11 p.m., Ian Downes wrote:
> > src/linux/routing/queueing/statistics.hpp, lines 26-34
> > <https://reviews.apache.org/r/34426/diff/8/?file=975302#file975302line26>
> >
> > constexpr?
Nice :)
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86058
-----------------------------------------------------------
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86058
-----------------------------------------------------------
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment137934>
Find out what? Drop the clause or be specific, please. If the link doesn't exist is Result more appropriate so it can return None?
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137932>
+1 If we have a hard requirement for libnl >= 3.2.26 then let's not do this.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137929>
s/q/Q
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137930>
ditto
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137924>
use foreachpair and don't use auto where it's useful to see the type (and it's a short type name).
src/linux/routing/queueing/statistics.hpp
<https://reviews.apache.org/r/34426/#comment137926>
constexpr?
src/linux/routing/queueing/statistics.cpp
<https://reviews.apache.org/r/34426/#comment137927>
ditto
- Ian Downes
On May 31, 2015, 12:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 12:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
> On June 1, 2015, 6:22 p.m., Chi Zhang wrote:
> > src/linux/routing/queueing/fq_codel.hpp, lines 63-64
> > <https://reviews.apache.org/r/34426/diff/8/?file=975297#file975297line63>
> >
> > nit: can probably save the "or an error if we cannot find out"
Disagree, I am trying to provide guidance between returning Error() and false.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86022
-----------------------------------------------------------
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86022
-----------------------------------------------------------
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment137863>
nit: can probably save the "or an error if we cannot find out"
src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/34426/#comment137865>
ditto.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137870>
Maybe a TODO for now.
we should check (ifdef?) on the version to try to use the libnl conversion function as this table can change in the future.
We do this for glibc in the codebase to patch in the symbols nonexistent in older glibcs
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137872>
Return None to be consistent with the other APIs? (Try -> Result)
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137876>
Here we can just return the result if it's not some.
- Chi Zhang
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated June 4, 2015, 1:19 a.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Incorporated review feedback.
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics from libnl
Diffs (updated)
-----
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
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
> 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
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Jie Yu <yu...@gmail.com>.
> 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 <=?
>
> Paul Brett wrote:
> 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.
>
> Cong Wang wrote:
> This looks like a libnl bug for me, however it is too late to fix it...
OK, I added a NOTE for you because this is very unintuitive.
> 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?
>
> Paul Brett wrote:
> 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.
SGTM.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86493
-----------------------------------------------------------
On June 4, 2015, 1:19 a.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated June 4, 2015, 1:19 a.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
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Cong Wang <cw...@twopensource.com>.
> 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 <=?
>
> Paul Brett wrote:
> 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.
This looks like a libnl bug for me, however it is too late to fix it...
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86493
-----------------------------------------------------------
On June 4, 2015, 1:19 a.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated June 4, 2015, 1:19 a.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
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86493
-----------------------------------------------------------
Ship it!
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment138537>
I would probably just use 'size_t' here:
```
for (size_t i = 0; i < (size_t) RTNL_TC_STATS_MAX; i++) {
...
}
```
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment138539>
Why <=?
src/linux/routing/queueing/statistics.hpp
<https://reviews.apache.org/r/34426/#comment138540>
Should we get rid of 'queueing' here because this can be used by filters as well?
- Jie Yu
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
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
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.
Changes
-------
Address reviewers comments and update to new qdisc internal interface.
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics from libnl
Diffs (updated)
-----
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
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated June 3, 2015, 7:36 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Rebase and update to match new discipline interface.
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics from libnl
Diffs (updated)
-----
src/Makefile.am f045b89e8590c4775b21980bedff14cc27d31bfb
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
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated June 2, 2015, 4:31 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Fix rebase
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics from libnl
Diffs (updated)
-----
src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90
src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
src/linux/routing/queueing/fq_codel.cpp 4dc2a9d2ed52937f0a78a083980db488c06b45a7
src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
> On June 1, 2015, 6:44 p.m., Cong Wang wrote:
> > src/linux/routing/queueing/internal.hpp, line 284
> > <https://reviews.apache.org/r/34426/diff/8/?file=975301#file975301line284>
> >
> > The lastest mesos code already relies on libnl 3.2.26, so not sure how much sense it makes to keep the backward compatibility here.
3.2.26 is not yet required as we have not pushed the last of the fq_codel changes to head. Lets leave this as a TODO until the dependency is committed to master.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86040
-----------------------------------------------------------
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Cong Wang <cw...@twopensource.com>.
> On June 1, 2015, 6:44 p.m., Cong Wang wrote:
> > src/linux/routing/queueing/internal.hpp, line 284
> > <https://reviews.apache.org/r/34426/diff/8/?file=975301#file975301line284>
> >
> > The lastest mesos code already relies on libnl 3.2.26, so not sure how much sense it makes to keep the backward compatibility here.
>
> Paul Brett wrote:
> 3.2.26 is not yet required as we have not pushed the last of the fq_codel changes to head. Lets leave this as a TODO until the dependency is committed to master.
rtnl_u32_get_classid() is added in 3.2.26 and it has been already in master branch.
- Cong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86040
-----------------------------------------------------------
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review86040
-----------------------------------------------------------
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment137892>
The lastest mesos code already relies on libnl 3.2.26, so not sure how much sense it makes to keep the backward compatibility here.
- Cong Wang
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 31, 2015, 7:53 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Brake out the components of the qdisc statistics for easier review.
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics from libnl
Diffs (updated)
-----
src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
src/linux/routing/queueing/statistics.hpp PRE-CREATION
src/linux/routing/queueing/statistics.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 27, 2015, 9:03 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/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
> On May 26, 2015, 10:56 p.m., Chi Zhang wrote:
> > src/linux/routing/queueing/fq_codel.cpp, line 80
> > <https://reviews.apache.org/r/34426/diff/7/?file=969377#file969377line80>
> >
> > would egress::ROOT be more consistent?
Yes, but it would require adding a namespace to hold a single value.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review85274
-----------------------------------------------------------
On May 31, 2015, 7:53 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 31, 2015, 7:53 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 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241
> src/linux/routing/queueing/fq_codel.hpp 7de1e31d4c43ac9cbffab1e472ea51140719f900
> src/linux/routing/queueing/fq_codel.cpp 64b5c73e8cfa672141ddb663e879c58b4babfdbc
> src/linux/routing/queueing/ingress.hpp 84506fecd01522471a7998176c28bea8f1367aed
> src/linux/routing/queueing/ingress.cpp ae0c38d2215e7fabcc1060e7385484bd455e1699
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
> src/linux/routing/queueing/statistics.hpp PRE-CREATION
> src/linux/routing/queueing/statistics.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review85274
-----------------------------------------------------------
the nits below are really not so big a deal, but I'd like to see smaller patches too..
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment136822>
maybe mention the added parameters in the comments? up to you.
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment136820>
nit" s/exists/exist/
src/linux/routing/queueing/fq_codel.cpp
<https://reviews.apache.org/r/34426/#comment136824>
would egress::ROOT be more consistent?
src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/34426/#comment136827>
nit: s/handles/handle/
src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/34426/#comment136828>
s/exists/exist/
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136831>
update the comment as well?
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136834>
the old way should be the correct style
- Chi Zhang
On May 22, 2015, 4:31 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 4:31 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/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
> src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
> src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
> src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
> src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
> src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
> src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
> src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review84830
-----------------------------------------------------------
Thanks for the efforts! Here are my main sugguestions for this patch:
1) Please split it out. Let's do step by step.
2) Introduce a top level Decipline (similar to Filter).
Please let me know if you want to chat about that.
src/linux/routing/queueing/ingress.cpp
<https://reviews.apache.org/r/34426/#comment136239>
Do you have this defined somewhere already?
src/linux/routing/queueing/ingress.cpp
<https://reviews.apache.org/r/34426/#comment136251>
First of all, I think fixing the ingress handle is OK for now. Second, if you really want to change this, you should probably do that in a separate patch.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136231>
These causes unnessary code jumping and it's bad for readability. Could you please revert this change? Thanks!
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136241>
I prefer keeping this function simple (just getting all the qdiscs). The filtering can be handled by `getQdisc`.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136242>
As I mentioned above, Let's keep getQdiscs simple and put all the filtering logic in the following for loop.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136389>
Can this be in a separate patch?
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136393>
Hum.. Looking at those interfaces make me feel that we should probably introduce a top level Discipline class (not the same as the current one, but similar to Filter):
```
template <typename Config>
class Discipline
{
Handle parent_;
Option<Handle> handle_;
string kind_;
Config config_;
};
```
And the 'create' interface should be:
```
template <typename Config>
Try<bool> create(
const std::string& _link,
const Discipline<Config>& discipline)
{
...
}
```
Let's try not to return the Handle in this patch! Keep this patch smaller makes it easier for the reviewers!
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment136250>
? Please do not sneak in changes like this in an already very huge diff.
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/34426/#comment136394>
Let's still make the ingress::HANDLE fixed so that we don't need to change port mapping isolator in this patch.
- Jie Yu
On May 22, 2015, 4:31 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 4:31 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/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
> src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
> src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
> src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
> src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
> src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
> src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
> src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 22, 2015, 4:31 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Update to reflect changes in Handle
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description (updated)
-------
Report the network statistics from libnl
Diffs (updated)
-----
src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 21, 2015, 5:52 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Backout bad patch update
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description
-------
Report the network statistics
Diffs (updated)
-----
src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 21, 2015, 5:45 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
Diffs (updated)
-----
include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f
src/slave/containerizer/isolators/network/port_mapping.hpp c72fb47f60f40cda8d84a10497b9133f83cf018e
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Report per-container metrics for network
bandwidth throttling
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 20, 2015, 11:44 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
Changes
-------
Dependent changes are now review in separate commits
Summary (updated)
-----------------
Report per-container metrics for network bandwidth throttling
Bugs: MESOS-2665
https://issues.apache.org/jira/browse/MESOS-2665
Repository: mesos
Description (updated)
-------
Report the network statistics
Diffs (updated)
-----
include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f
src/slave/containerizer/isolators/network/port_mapping.hpp c72fb47f60f40cda8d84a10497b9133f83cf018e
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Extend the Traffic Control queueing models
to allow dynamically allocated Handles,
extend API to allow searching by handle (or parent).
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 20, 2015, 5:55 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
-------
Extend the Traffic Control queueing models to allow dynamically allocated Handles, extend API to allow searching by handle (or parent).
Diffs (updated)
-----
src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 34426: Extend the Traffic Control queueing models
to allow dynamically allocated Handles,
extend API to allow searching by handle (or parent).
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/#review84445
-----------------------------------------------------------
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment135709>
It doesn't return false, it returns a Handle.
src/linux/routing/queueing/fq_codel.hpp
<https://reviews.apache.org/r/34426/#comment135711>
Please reorder the parameters, move 'parent' before 'handle', this reads better.
src/linux/routing/queueing/fq_codel.cpp
<https://reviews.apache.org/r/34426/#comment135712>
Ditto
src/linux/routing/queueing/ingress.hpp
<https://reviews.apache.org/r/34426/#comment135708>
The comment needs to update, since now the return value is Handle.
src/linux/routing/queueing/ingress.cpp
<https://reviews.apache.org/r/34426/#comment135715>
Please print the link name in error message.
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment135717>
Ditto
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34426/#comment135710>
Ditto
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/34426/#comment135713>
s/the filter handles/the ingress qdisc handle/ ?
src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/34426/#comment135718>
Ditto
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34426/#comment135725>
s/eth0IngressQdisc/eth0IngressHandle/
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34426/#comment135726>
Ditto
src/tests/routing_tests.cpp
<https://reviews.apache.org/r/34426/#comment135719>
vethHandle is a bad name, it reads like a handle of veth. Please rename it to 'egressHandle'.
- Cong Wang
On May 19, 2015, 7:55 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34426/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 7:55 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
> -------
>
> Extend the Traffic Control queueing models to allow dynamically allocated Handles, extend API to allow searching by handle (or parent).
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
> src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
> src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
> src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
> src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
> src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
> src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
> src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
>
> Diff: https://reviews.apache.org/r/34426/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 34426: Extend the Traffic Control queueing models
to allow dynamically allocated Handles,
extend API to allow searching by handle (or parent).
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34426/
-----------------------------------------------------------
(Updated May 19, 2015, 7:55 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
-------
Extend the Traffic Control queueing models to allow dynamically allocated Handles, extend API to allow searching by handle (or parent).
Diffs (updated)
-----
src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00
src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb
src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b
src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba
src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69
src/slave/containerizer/isolators/network/port_mapping.cpp 49e983edab598e2ac487bb488fdd12840a9e7dfc
src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502
src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa
Diff: https://reviews.apache.org/r/34426/diff/
Testing
-------
make check
Thanks,
Paul Brett