You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/06/03 00:04:29 UTC
Review Request 34956: Refactored the queueing discipline data
structure.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
Repository: mesos
Description
-------
Refactored the queueing discipline data structure.
This is for us to more easily extend qdisc support.
Diffs
-----
src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
src/linux/routing/queueing/discipline.hpp PRE-CREATION
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/34956/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 34956: Refactored the queueing discipline data
structure.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/#review86336
-----------------------------------------------------------
src/linux/routing/queueing/discipline.hpp
<https://reviews.apache.org/r/34956/#comment138293>
Any reason for the const getter boilerplate? From what I can tell, it looks like you have a `const Discipline` in your code already.
I think we've avoided adding getters like this in general for simple structs (which this looks like to me), so it seems a bit inconsistent?
- Ben Mahler
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:04 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored the queueing discipline data structure.
>
> This is for us to more easily extend qdisc support.
>
>
> Diffs
> -----
>
> src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
> src/linux/routing/queueing/discipline.hpp PRE-CREATION
> 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/34956/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34956: Refactored the queueing discipline data
structure.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/#review86313
-----------------------------------------------------------
Ship it!
Ship It!
- Paul Brett
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:04 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored the queueing discipline data structure.
>
> This is for us to more easily extend qdisc support.
>
>
> Diffs
> -----
>
> src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
> src/linux/routing/queueing/discipline.hpp PRE-CREATION
> 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/34956/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34956: Refactored the queueing discipline data
structure.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2015, 10:40 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/discipline.hpp, line 39
> > <https://reviews.apache.org/r/34956/diff/1/?file=976864#file976864line39>
> >
> > Should config be optional?
>
> Jie Yu wrote:
> We still need a Config struct for each type of qdisc so that the template specification could work. It just looks weired to me to do
> ```
> Discipline<ingress::Config>(
> ...
> None());
> ```
>
> Do you have a strong opinion on that?
Synced with Paul offline. Decided to punt this for now. I'll add a TODO.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/#review86300
-----------------------------------------------------------
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:04 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored the queueing discipline data structure.
>
> This is for us to more easily extend qdisc support.
>
>
> Diffs
> -----
>
> src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
> src/linux/routing/queueing/discipline.hpp PRE-CREATION
> 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/34956/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34956: Refactored the queueing discipline data
structure.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2015, 10:40 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/discipline.hpp, line 39
> > <https://reviews.apache.org/r/34956/diff/1/?file=976864#file976864line39>
> >
> > Should config be optional?
We still need a Config struct for each type of qdisc so that the template specification could work. It just looks weired to me to do
```
Discipline<ingress::Config>(
...
None());
```
Do you have a strong opinion on that?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/#review86300
-----------------------------------------------------------
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:04 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored the queueing discipline data structure.
>
> This is for us to more easily extend qdisc support.
>
>
> Diffs
> -----
>
> src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
> src/linux/routing/queueing/discipline.hpp PRE-CREATION
> 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/34956/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34956: Refactored the queueing discipline data
structure.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956/#review86300
-----------------------------------------------------------
src/linux/routing/queueing/discipline.hpp
<https://reviews.apache.org/r/34956/#comment138279>
Should config be optional?
- Paul Brett
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:04 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored the queueing discipline data structure.
>
> This is for us to more easily extend qdisc support.
>
>
> Diffs
> -----
>
> src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
> src/linux/routing/queueing/discipline.hpp PRE-CREATION
> 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/34956/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>