You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2014/06/19 21:30:40 UTC
Review Request 22777: Added support for optionally throttling the frameworks
not specified in RateLimits config.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-1492
https://issues.apache.org/jira/browse/MESOS-1492
Repository: mesos-git
Description
-------
In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
Diffs
-----
include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
src/master/master.hpp 2844446e2674df6e11f4c2915ed324fcc103532c
src/master/master.cpp 888657dd4bc50085882382908e3c48ccb857c621
src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
Diff: https://reviews.apache.org/r/22777/diff/
Testing
-------
Added/modified tests for the default limiter.
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/#review46329
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81653>
s/principals/principal/
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81654>
Is this true? What if this is a registration message from a framework?
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81655>
we don't do consts for bools/ints.
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81657>
s/exists/exist/
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81658>
s/for//
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/22777/#comment81663>
i'll look at these tests after you fix them in the other review.
- Vinod Kone
On June 19, 2014, 7:30 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22777/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 7:30 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1492
> https://issues.apache.org/jira/browse/MESOS-1492
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
>
> We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
>
> This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
> src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
> src/master/master.hpp 2844446e2674df6e11f4c2915ed324fcc103532c
> src/master/master.cpp 888657dd4bc50085882382908e3c48ccb857c621
> src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
>
> Diff: https://reviews.apache.org/r/22777/diff/
>
>
> Testing
> -------
>
> Added/modified tests for the default limiter.
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/#review46416
-----------------------------------------------------------
Ship it!
include/mesos/mesos.proto
<https://reviews.apache.org/r/22777/#comment81806>
s/them./them,/
src/master/master.cpp
<https://reviews.apache.org/r/22777/#comment81807>
s/about/with/
- Vinod Kone
On June 23, 2014, 4:12 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22777/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 4:12 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1492
> https://issues.apache.org/jira/browse/MESOS-1492
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
>
> We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
>
> This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
> src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
> src/master/master.hpp f4cdb26567a316755dbb55da239277bb370bf696
> src/master/master.cpp 00152f5b889a2d10255835779b66c9ed26df9a2b
> src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
>
> Diff: https://reviews.apache.org/r/22777/diff/
>
>
> Testing
> -------
>
> Added/modified tests for the default limiter.
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/
-----------------------------------------------------------
(Updated June 23, 2014, 11:15 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Vinod's comments. NNFR.
Bugs: MESOS-1492
https://issues.apache.org/jira/browse/MESOS-1492
Repository: mesos-git
Description
-------
In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
src/master/master.hpp f4cdb26567a316755dbb55da239277bb370bf696
src/master/master.cpp 00152f5b889a2d10255835779b66c9ed26df9a2b
src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
Diff: https://reviews.apache.org/r/22777/diff/
Testing
-------
Added/modified tests for the default limiter.
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/
-----------------------------------------------------------
(Updated June 22, 2014, 9:12 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-1492
https://issues.apache.org/jira/browse/MESOS-1492
Repository: mesos-git
Description
-------
In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
Diffs
-----
include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
src/master/master.hpp f4cdb26567a316755dbb55da239277bb370bf696
src/master/master.cpp 00152f5b889a2d10255835779b66c9ed26df9a2b
src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
Diff: https://reviews.apache.org/r/22777/diff/
Testing
-------
Added/modified tests for the default limiter.
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/
-----------------------------------------------------------
(Updated June 22, 2014, 9:12 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Vinod's comments.
Bugs: MESOS-1492
https://issues.apache.org/jira/browse/MESOS-1492
Repository: mesos-git
Description
-------
In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
Diffs (updated)
-----
include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
src/master/master.hpp f4cdb26567a316755dbb55da239277bb370bf696
src/master/master.cpp 00152f5b889a2d10255835779b66c9ed26df9a2b
src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
Diff: https://reviews.apache.org/r/22777/diff/
Testing
-------
Added/modified tests for the default limiter.
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 20, 2014, 9:27 a.m., Dominic Hamon wrote:
> > include/mesos/mesos.proto, line 780
> > <https://reviews.apache.org/r/22777/diff/1/?file=613113#file613113line780>
> >
> > this API is counter-intuitive. I would expect, because limits are set per-principle, that this default qps would also be applied per-principle.
> >
> > Is there a technical reason why we can't set a limiter up for every framework we encounter with the qps set to a limit (if in limits) or the default (if not)?
This default is used to safeguard the Master by limiting unexpected "framework traffic" the operator doesn't know beforehand but still would like to allow.
Having a default per principal doesn't achieve this goal because Master could end up giving out more bandwidth if there are many unexpected frameworks connecting to it.
We tried to come up with a very descriptive name for this but so far this is the best we've got.
Open to suggestions.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/#review46284
-----------------------------------------------------------
On June 19, 2014, 12:30 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22777/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 12:30 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1492
> https://issues.apache.org/jira/browse/MESOS-1492
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
>
> We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
>
> This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
> src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
> src/master/master.hpp 2844446e2674df6e11f4c2915ed324fcc103532c
> src/master/master.cpp 888657dd4bc50085882382908e3c48ccb857c621
> src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
>
> Diff: https://reviews.apache.org/r/22777/diff/
>
>
> Testing
> -------
>
> Added/modified tests for the default limiter.
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22777: Added support for optionally throttling the
frameworks not specified in RateLimits config.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22777/#review46284
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/22777/#comment81593>
this API is counter-intuitive. I would expect, because limits are set per-principle, that this default qps would also be applied per-principle.
Is there a technical reason why we can't set a limiter up for every framework we encounter with the qps set to a limit (if in limits) or the default (if not)?
- Dominic Hamon
On June 19, 2014, 12:30 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22777/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 12:30 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1492
> https://issues.apache.org/jira/browse/MESOS-1492
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> In current previous design and implementation of framework rate limiting, Master only throttles frameworks specified explicitly in RateLimits. Frameworks not specified are not throttled.
>
> We can add an {{optional double aggregate_default_qps}} to RateLimits protobuf and throttle all of the unspecified frameworks together using one RateLimiter at the {{aggregate_default_qps}} rate.
>
> This allows the Mesos cluster operator to safeguard the Master from unexpected frameworks and it's optional.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2f6be059d3626663b73417863b42505c36f0446f
> src/master/flags.hpp 47bb0dc04504fc847cc02fa97eb0d045a8835c9e
> src/master/master.hpp 2844446e2674df6e11f4c2915ed324fcc103532c
> src/master/master.cpp 888657dd4bc50085882382908e3c48ccb857c621
> src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9
>
> Diff: https://reviews.apache.org/r/22777/diff/
>
>
> Testing
> -------
>
> Added/modified tests for the default limiter.
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>