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/10 20:15:19 UTC
Review Request 22425: Created framework rate limits protobuf object which is
loaded as JSON through master flags
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
Bugs: MESOS-1443
https://issues.apache.org/jira/browse/MESOS-1443
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
Diff: https://reviews.apache.org/r/22425/diff/
Testing
-------
make check (tested along with https://reviews.apache.org/r/22427)
Thanks,
Jiang Yan Xu
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Vinod Kone <vi...@gmail.com>.
> On June 10, 2014, 8:37 p.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, line 669
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line669>
> >
> > Another option is to just have the users set both "permits" and "duration". Then you dont need to worry about precision. Not sure how user friendly is that though. Have you looked around at examples on the interwebs?
>
> Jiang Yan Xu wrote:
> Not configuration examples for end products but Guava's RateLimiter takes a "double permitsPerSecond" in its creator method. Perhaps we can use that for our RateLimiter as well? It's internally converted to a 'double rate'. We can just store the 'rate' and add an overload constructor for RateLimiter that takes a "double permitsPerSecond". What do you think?
sgtm. @benh: any objections?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45281
-----------------------------------------------------------
On June 10, 2014, 6:15 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 10, 2014, 1:37 p.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, line 669
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line669>
> >
> > Another option is to just have the users set both "permits" and "duration". Then you dont need to worry about precision. Not sure how user friendly is that though. Have you looked around at examples on the interwebs?
Not configuration examples for end products but Guava's RateLimiter takes a "double permitsPerSecond" in its creator method. Perhaps we can use that for our RateLimiter as well? It's internally converted to a 'double rate'. We can just store the 'rate' and add an overload constructor for RateLimiter that takes a "double permitsPerSecond". What do you think?
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45281
-----------------------------------------------------------
On June 10, 2014, 11:15 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 11:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45281
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80042>
Another option is to just have the users set both "permits" and "duration". Then you dont need to worry about precision. Not sure how user friendly is that though. Have you looked around at examples on the interwebs?
src/master/flags.hpp
<https://reviews.apache.org/r/22425/#comment80036>
s/aurora/foo/
src/master/flags.hpp
<https://reviews.apache.org/r/22425/#comment80037>
s/jenkins/bar/
src/master/master.cpp
<https://reviews.apache.org/r/22425/#comment80039>
Change it to "Enabled rate limiting". Authorization log line is changed similarly in my current chain of review.s
src/master/master.cpp
<https://reviews.apache.org/r/22425/#comment80040>
say "(see --rate_limits flag)"
- Vinod Kone
On June 10, 2014, 6:15 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 6:15 p.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 10, 2014, 11:32 a.m., Dominic Hamon wrote:
> > include/mesos/mesos.proto, line 669
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line669>
> >
> > It's not part of this review, but where (and why) does this ignoring happen?
>
> Jiang Yan Xu wrote:
> So the RateLimiter constructor takes an integer and a duration: RateLimiter(int permits, const Duration& duration);
> The configuration is in 'double' QPS.
>
> Converting the 'double QPS' into 'int permits' and 'duration' pair that maintains the full precision of the 'double' value is not valuable enough to justify the effort IMO so I chose to truncate the QPS value and give the user a warning here...
>
> Thoughts?
>
> Dominic Hamon wrote:
> I understand, and that makes sense. How did you decide on 0.01? That works out to 1 query every 100 seconds which seems like a very slow rate.
>
> Is it worth adding logging where you do the truncation if the user has tried to specify something with more precision or, more likely, something less than 0.01 that is then truncated to 0?
OK I meant that e.g. "500000" vs "500001" queries every 100 seconds (5000 vs 5000.01 QPS) is as much as we can distinguish and we don't distinguish "5000002" and "5000004" queries per 1000 seconds (5000.002 vs 5000.004 QPS).
The 0.01 precision is what I thought reasonable but I guess a few more precision digits are fine? :)
Yes I agree I should add logging.
Perhaps I should throw an error when the specified QPS is below 0.01 (and certainly for zero and negative values as well)
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45266
-----------------------------------------------------------
On June 10, 2014, 11:15 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 11:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 10, 2014, 11:32 a.m., Dominic Hamon wrote:
> > include/mesos/mesos.proto, line 672
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line672>
> >
> > s/of be/to be/
Done.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45266
-----------------------------------------------------------
On June 11, 2014, 10:39 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 10:39 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 102289b07b5704c70ae56e2e29dca6dd737c8ec2
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 10, 2014, 11:32 a.m., Dominic Hamon wrote:
> > include/mesos/mesos.proto, line 669
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line669>
> >
> > It's not part of this review, but where (and why) does this ignoring happen?
So the RateLimiter constructor takes an integer and a duration: RateLimiter(int permits, const Duration& duration);
The configuration is in 'double' QPS.
Converting the 'double QPS' into 'int permits' and 'duration' pair that maintains the full precision of the 'double' value is not valuable enough to justify the effort IMO so I chose to truncate the QPS value and give the user a warning here...
Thoughts?
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45266
-----------------------------------------------------------
On June 10, 2014, 11:15 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 11:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Dominic Hamon <dh...@twopensource.com>.
> On June 10, 2014, 11:32 a.m., Dominic Hamon wrote:
> > include/mesos/mesos.proto, line 669
> > <https://reviews.apache.org/r/22425/diff/1/?file=606515#file606515line669>
> >
> > It's not part of this review, but where (and why) does this ignoring happen?
>
> Jiang Yan Xu wrote:
> So the RateLimiter constructor takes an integer and a duration: RateLimiter(int permits, const Duration& duration);
> The configuration is in 'double' QPS.
>
> Converting the 'double QPS' into 'int permits' and 'duration' pair that maintains the full precision of the 'double' value is not valuable enough to justify the effort IMO so I chose to truncate the QPS value and give the user a warning here...
>
> Thoughts?
I understand, and that makes sense. How did you decide on 0.01? That works out to 1 query every 100 seconds which seems like a very slow rate.
Is it worth adding logging where you do the truncation if the user has tried to specify something with more precision or, more likely, something less than 0.01 that is then truncated to 0?
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45266
-----------------------------------------------------------
On June 10, 2014, 11:15 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 11:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45266
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80009>
It's not part of this review, but where (and why) does this ignoring happen?
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80007>
s/of be/to be/
- Dominic Hamon
On June 10, 2014, 11:15 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 11:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 62f69d26ba0132156081eb7cfd0f16dd1b22913d
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f
> src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Vinod Kone <vi...@gmail.com>.
> On June 14, 2014, 12:28 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 688-692
> > <https://reviews.apache.org/r/22425/diff/3/?file=609705#file609705line688>
> >
> > It would be nice to allow operators to explicitly specify default QPS for principals that are not specified in the limits.
> >
> > We could do this pretty easily by adding an optional 'default_qps' field here:
> >
> > // QPS for unspecified principals, if left unset, then no rate limit will be enforced.
> > optional default_qps = 2;
>
> Jiang Yan Xu wrote:
> Should all frameworks with unspecified principals be throttled together? i.e. should they each get default_qps or they should collectively get default_qps?
>
> I think the name 'default_qps' intuitively suggests the former but from the operator's POV, it's more useful if it's the latter because this effectively limits the unexpected traffic.
>
> And this can probably be in its own patch with the Master logic together.
The latter sounds good. Just make sure you clearly comment that in the proto.
Also, this patch is straightforward. So, just add the "default_qps" in this patch instead of splitting the protobuf format across separate reviews.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45681
-----------------------------------------------------------
On June 13, 2014, 11 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 11 p.m.)
>
>
> Review request for mesos, Dominic Hamon and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 13, 2014, 5:28 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 688-692
> > <https://reviews.apache.org/r/22425/diff/3/?file=609705#file609705line688>
> >
> > It would be nice to allow operators to explicitly specify default QPS for principals that are not specified in the limits.
> >
> > We could do this pretty easily by adding an optional 'default_qps' field here:
> >
> > // QPS for unspecified principals, if left unset, then no rate limit will be enforced.
> > optional default_qps = 2;
>
> Jiang Yan Xu wrote:
> Should all frameworks with unspecified principals be throttled together? i.e. should they each get default_qps or they should collectively get default_qps?
>
> I think the name 'default_qps' intuitively suggests the former but from the operator's POV, it's more useful if it's the latter because this effectively limits the unexpected traffic.
>
> And this can probably be in its own patch with the Master logic together.
>
> Vinod Kone wrote:
> The latter sounds good. Just make sure you clearly comment that in the proto.
>
> Also, this patch is straightforward. So, just add the "default_qps" in this patch instead of splitting the protobuf format across separate reviews.
To summarize the offline chat with Vinod and BenM:
1) Name this optional "default" variable "aggregate_default_qps": all the frameworks not specified in RateLimits config are throttled together using on RateLimiter if this variable is set.
2) For frameworks you want to grant unlimited bandwidth explicitly, set the 'principal' but do not set the 'qps' (will make it optional)
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45681
-----------------------------------------------------------
On June 13, 2014, 4 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 4 p.m.)
>
>
> Review request for mesos, Dominic Hamon and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On June 13, 2014, 5:28 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 688-692
> > <https://reviews.apache.org/r/22425/diff/3/?file=609705#file609705line688>
> >
> > It would be nice to allow operators to explicitly specify default QPS for principals that are not specified in the limits.
> >
> > We could do this pretty easily by adding an optional 'default_qps' field here:
> >
> > // QPS for unspecified principals, if left unset, then no rate limit will be enforced.
> > optional default_qps = 2;
Should all frameworks with unspecified principals be throttled together? i.e. should they each get default_qps or they should collectively get default_qps?
I think the name 'default_qps' intuitively suggests the former but from the operator's POV, it's more useful if it's the latter because this effectively limits the unexpected traffic.
And this can probably be in its own patch with the Master logic together.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45681
-----------------------------------------------------------
On June 13, 2014, 4 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 4 p.m.)
>
>
> Review request for mesos, Dominic Hamon and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45681
-----------------------------------------------------------
Ship it!
Vinod and I went over this, just a few comments below about naming and having the ability to specify a default if desired.
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80621>
How about calling this just 'qps', it's not clear what meaning 'maximum' may have in the future as we consider bursting capabilities?
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80622>
It would be nice to allow operators to explicitly specify default QPS for principals that are not specified in the limits.
We could do this pretty easily by adding an optional 'default_qps' field here:
// QPS for unspecified principals, if left unset, then no rate limit will be enforced.
optional default_qps = 2;
src/master/flags.hpp
<https://reviews.apache.org/r/22425/#comment80623>
Make sure to update this as you change the protobuf.
- Ben Mahler
On June 13, 2014, 11 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 11 p.m.)
>
>
> Review request for mesos, Dominic Hamon and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45670
-----------------------------------------------------------
Ship it!
Ship It!
- Dominic Hamon
On June 13, 2014, 4 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 4 p.m.)
>
>
> Review request for mesos, Dominic Hamon and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/#review45881
-----------------------------------------------------------
Ship it!
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80953>
s/Master/master/
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80954>
s/i.e./i.e.,/
include/mesos/mesos.proto
<https://reviews.apache.org/r/22425/#comment80955>
and Credential.principal (if using authentication) ?
- Vinod Kone
On June 17, 2014, 12:29 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22425/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1443
> https://issues.apache.org/jira/browse/MESOS-1443
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 709b8b1ef4fee17c6a95c37f9ca6ea6ccc2a5daf
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
> src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
>
> Diff: https://reviews.apache.org/r/22425/diff/
>
>
> Testing
> -------
>
> make check (tested along with https://reviews.apache.org/r/22427)
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/
-----------------------------------------------------------
(Updated June 17, 2014, 1:57 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Vinods'. NNFR.
Bugs: MESOS-1443
https://issues.apache.org/jira/browse/MESOS-1443
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 709b8b1ef4fee17c6a95c37f9ca6ea6ccc2a5daf
src/master/flags.hpp 7850e4597f8e94b5576a5dddd3f68b15741ce87b
src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
Diff: https://reviews.apache.org/r/22425/diff/
Testing
-------
make check (tested along with https://reviews.apache.org/r/22427)
Thanks,
Jiang Yan Xu
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/
-----------------------------------------------------------
(Updated June 16, 2014, 5:29 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
BenM's and Vinod's comments.
In the spirit of more rapid iterations on smaller improvements each time, I am leaving out the 'aggregate_default_qps' portion from this review because it adds complexity to the rate limiting logic in Master that hasn't been fully discussed. I have filed https://issues.apache.org/jira/browse/MESOS-1492 for it and will address it in the next iteration.
Summary (updated)
-----------------
Created framework rate limits protobuf object which is loaded as JSON through master flags.
Bugs: MESOS-1443
https://issues.apache.org/jira/browse/MESOS-1443
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 709b8b1ef4fee17c6a95c37f9ca6ea6ccc2a5daf
src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
Diff: https://reviews.apache.org/r/22425/diff/
Testing
-------
make check (tested along with https://reviews.apache.org/r/22427)
Thanks,
Jiang Yan Xu
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/
-----------------------------------------------------------
(Updated June 13, 2014, 4 p.m.)
Review request for mesos, Dominic Hamon and Vinod Kone.
Changes
-------
Rebased. No change for review but still need a shipit. More RateLimit validation is added in https://reviews.apache.org/r/22427 where RateLimiters are added.
Bugs: MESOS-1443
https://issues.apache.org/jira/browse/MESOS-1443
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 8c6933db495ef0bb3c14301d8077d31ecb123f59
src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
Diff: https://reviews.apache.org/r/22425/diff/
Testing
-------
make check (tested along with https://reviews.apache.org/r/22427)
Thanks,
Jiang Yan Xu
Re: Review Request 22425: Created framework rate limits protobuf object
which is loaded as JSON through master flags
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22425/
-----------------------------------------------------------
(Updated June 11, 2014, 10:39 a.m.)
Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
Changes
-------
Comments.
Bugs: MESOS-1443
https://issues.apache.org/jira/browse/MESOS-1443
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 102289b07b5704c70ae56e2e29dca6dd737c8ec2
src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59
src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09
Diff: https://reviews.apache.org/r/22425/diff/
Testing
-------
make check (tested along with https://reviews.apache.org/r/22427)
Thanks,
Jiang Yan Xu