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/08/06 01:41:17 UTC
Review Request 24343: Improved framework rate limiting by imposing the max
number of outstanding messages per framework principal.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 5, 2014, 11:25 p.m., Timothy Chen wrote:
> > src/master/master.hpp, line 766
> > <https://reviews.apache.org/r/24343/diff/2/?file=653520#file653520line766>
> >
> > It also returns error when the rate limiter reachs its capacity too right?
the rate limiter doesn't have a capacity.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49699
-----------------------------------------------------------
On Aug. 6, 2014, 4:13 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:13 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp a925a936bc1cd878f1c9ff63a9dcc4ee808ec7a5
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49699
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment86996>
It also returns error when the rate limiter reachs its capacity too right?
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment86988>
Nit: Seems like it's used just as a queue, why not just use queue?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment86992>
I wonder if we should rename __visit to something like dropMessage or failed that it's obvious it's the failed throttle/limiter path.
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/24343/#comment86993>
I think you can bring it back one line:
error(driver,
"Message dropped: Reached capacity: 2")
- Timothy Chen
On Aug. 6, 2014, 5:53 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 5:53 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49844
-----------------------------------------------------------
Ship it!
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87234>
s/be/being/
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87238>
Would this be simpler if you assign the correct rate limiter (specific or default) first?
Option<Owned<BoundedRateLimiter> > limiter;
if (use specificLimiter) {
limiter = specific limiter;
} else if (use defualtLimiter) {
limiter = defaultLimiter;
}
if (limiter.isSome()) {
...
} else {
_visit(event);
}
- Vinod Kone
On Aug. 7, 2014, 12:25 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 12:25 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 7, 2014, 5:33 p.m., Ben Mahler wrote:
> > I realize this is already committed, had a few questions around doing this more cleanly. Let me know your thoughts!
I appreciate these comments and let's chat about what could be done to improve this!
> On Aug. 7, 2014, 5:33 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 778-781
> > <https://reviews.apache.org/r/24343/diff/6/?file=654370#file654370line778>
> >
> > What is the difference between None() and no entry in the map? If there's no difference should we eliminate the Option? Otherwise, maybe a comment is necessary here?
None is "not throttled", no entry is "use the default throttler" (if one is configured). If no default throttler is configured, the two cases are same.
Therefore there is a difference and the Option cannot be eliminated.
> On Aug. 7, 2014, 5:33 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 762-776
> > <https://reviews.apache.org/r/24343/diff/6/?file=654370#file654370line762>
> >
> > Have you considered making this an 'EventLimiter'? Right now the Master has to do the capacity accounting correctly whenever it throttles something.
Yes, see earlier versions of the review: https://reviews.apache.org/r/24343/diff/2/#index_header
The reason the logic is put back into Master is that Master still needs to call a nextEvent() method to get the event (as in version 2 of the review) or at least to do the capacity accounting. Then there was this question of "what if the user of the nested class (i.e., Master) doesn't call the methods in EventThrottler according to the prescribed order". With a data/dumb struct the caller still needs to know not to manipulate it incorrectly but at least with a dumb struct it's obvious that the burden is on the caller.
I basically followed the convention in other structs nested in Master.
I am all for modularization given the ever increasing complexity in Master but the challenge here is that the EventThrottler calls a method in RateLimiter that returns a Future but it cannot defer the Future to itself because it's not a Process. Only master can provide the callback and thus inevitably needs to call into the class again.
We can definitely iteration on this (e.g., can EventThrottler be itself a Process and be responsible for throttling all frameworks?) and let's chat if you have good ideas.
> On Aug. 7, 2014, 5:33 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 944-950
> > <https://reviews.apache.org/r/24343/diff/6/?file=654371#file654371line944>
> >
> > What is the invariant here that ensures that the principal is still present in the 'limiters' map? Is there a way we can better enforce the 'const'ness of the limiters map? A clear comment would be nice. Maybe some CHECKs here as well?
> >
> > Per my comment above, if we moved both the
> > (1) message accounting, and
> > (2) ExitedEvent vs. MessageEvent throttling decision
> > into the 'BoundedRateLimiter', then no accounting logic would be needed here.
> >
> > We could use failed Futures to represent being over capacity as well, simplifying the logic you have above.
> >
> > Any reason that you didn't chose to do it this way?
You are right! The CHECKs unfortunately weren't added in this review but they were in a subsequent one: https://github.com/apache/mesos/commit/8c4f45d67be22cfe252ad6ed27a79ad4a1f972c6
The limiters are static (never modified after initialization) so I thought it was clear. Of course I could add a comment here.
> On Aug. 7, 2014, 5:33 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 940
> > <https://reviews.apache.org/r/24343/diff/6/?file=654371#file654371line940>
> >
> > Any reason to call this throttled? This looks like _visit and _visit looks like __visit:
> >
> > Exited path: visit(ExitedEvent) -> _visit(ExitedEvent)
> > Message path: visit(MessageEvent) -> throttled(MessageEvent) -> _visit(MessageEvent)
> >
> > A more typical continuation naming scheme here would be:
> >
> > Exited path: visit(ExitedEvent) -> _visit(ExitedEvent)
> > Message path: visit(MessageEvent) -> _visit(MessageEvent) -> __visit(MessageEvent)
> >
> > This exposes the flow more clearly, no? But! Per my comment below, I think we can eliminate the need for 'throttled' entirely if we encapsulate the queue accounting inside the 'EventLimiter' abstraction. Anything I'm missing?
There is also a "capacity exceeded" path.
I was using the scheme you suggested but if you see Vinod's and Tim's comment, there were a few issues raised:
1. What constitutes a "continuation" and should use the underscore prefix scheme (e.g., whether it needs to be a deferred callback).
2. If a method clearly serves a single purpose that can be clearly summarized by one word, should we should a more explicit name?
We haven't been super consistent with the naming scheme but I think we started to do this when it was clear that we were going to start using lambdas soon to replace these continuation whenever appropriate. And all these methods groups are sequential that they "would have been written as one if it weren't because of asynchronous" I'd love to revisit this pattern later when we refactor.
In my case there are three alternative flows so it's not super clear prepending underscores helps with clarity but at least in the current implementation both _visit(MessageEvent) and _visit(ExitedEvent) are the 'real' message "visit"s and "throttled" merely does accounting so there is some consistency there.
See my comments above about EventLimiter.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49979
-----------------------------------------------------------
On Aug. 6, 2014, 11:26 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 11:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49979
-----------------------------------------------------------
I realize this is already committed, had a few questions around doing this more cleanly. Let me know your thoughts!
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87432>
Have you considered making this an 'EventLimiter'? Right now the Master has to do the capacity accounting correctly whenever it throttles something.
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87436>
What is the difference between None() and no entry in the map? If there's no difference should we eliminate the Option? Otherwise, maybe a comment is necessary here?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87439>
Any reason to call this throttled? This looks like _visit and _visit looks like __visit:
Exited path: visit(ExitedEvent) -> _visit(ExitedEvent)
Message path: visit(MessageEvent) -> throttled(MessageEvent) -> _visit(MessageEvent)
A more typical continuation naming scheme here would be:
Exited path: visit(ExitedEvent) -> _visit(ExitedEvent)
Message path: visit(MessageEvent) -> _visit(MessageEvent) -> __visit(MessageEvent)
This exposes the flow more clearly, no? But! Per my comment below, I think we can eliminate the need for 'throttled' entirely if we encapsulate the queue accounting inside the 'EventLimiter' abstraction. Anything I'm missing?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87435>
What is the invariant here that ensures that the principal is still present in the 'limiters' map? Is there a way we can better enforce the 'const'ness of the limiters map? A clear comment would be nice. Maybe some CHECKs here as well?
Per my comment above, if we moved both the
(1) message accounting, and
(2) ExitedEvent vs. MessageEvent throttling decision
into the 'BoundedRateLimiter', then no accounting logic would be needed here.
We could use failed Futures to represent being over capacity as well, simplifying the logic you have above.
Any reason that you didn't chose to do it this way?
- Ben Mahler
On Aug. 7, 2014, 6:26 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 6:26 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
(Updated Aug. 6, 2014, 11:26 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Vinod's comments. No need for review.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
(Updated Aug. 6, 2014, 5:25 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Now use uint64_t instead of size_t to keep track of capacity and number of outstanding messages.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 6, 2014, 4:53 p.m., Dominic Hamon wrote:
> > src/master/master.hpp, line 770
> > <https://reviews.apache.org/r/24343/diff/3/?file=654314#file654314line770>
> >
> > this should either be a uint64 (to match the proto) or the proto should be a uint32 (to be reasonable). There's no real benefit to storing this as a size_t.
> >
> > i'll note that we claim to follow the google c++ style guide which states that this should really just be an int unless we know we want to support more than 2^31 messages (unlikely): http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Integer_Types
> >
> >
thanks. the code used to check this against the queue.size() so size_t was used. not anymore. so I can just use uint64_t.
I think uint64 is more future proof and the style guide says "When in doubt, choose a larger type" :)
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49831
-----------------------------------------------------------
On Aug. 6, 2014, 4:37 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49831
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87210>
this should either be a uint64 (to match the proto) or the proto should be a uint32 (to be reasonable). There's no real benefit to storing this as a size_t.
i'll note that we claim to follow the google c++ style guide which states that this should really just be an int unless we know we want to support more than 2^31 messages (unlikely): http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Integer_Types
- Dominic Hamon
On Aug. 6, 2014, 4:37 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
(Updated Aug. 6, 2014, 4:37 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Rebased and removed some changes not belonged in this review.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp 97e4340f7949a261558f09ea533aac0bbb0e40f6
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
(Updated Aug. 6, 2014, 4:13 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Addressed comments.
Pulled the logic in EventThrottler into Master::visit() after discussion with Vinod because it is unable to enforce that the caller (i.e., Master) use its methods correctly (in the correct order).
Changed it to a dumb struct BoundedRateLimiter the same way other structs nested in Master are used.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp a925a936bc1cd878f1c9ff63a9dcc4ee808ec7a5
src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 375
> > <https://reviews.apache.org/r/24343/diff/2/?file=653521#file653521line375>
> >
> > Option<size_t> capacity;
> > if (limit_.has_capacity()) {
> > capacity = limit_.capacity();
> > }
> >
> > should work due to implicit conversion and is much clearer.
ok I agree. thanks.
I didn't remove the static_cast because it's more explicit and it might be a narrowing conversion. I added check for this case.
> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 394
> > <https://reviews.apache.org/r/24343/diff/2/?file=653521#file653521line394>
> >
> > see above
done
> On Aug. 6, 2014, 9:38 a.m., Dominic Hamon wrote:
> > src/master/master.hpp, line 756
> > <https://reviews.apache.org/r/24343/diff/2/?file=653520#file653520line756>
> >
> > it seems to me this could have some unit tests created outside of master tests. I'd really like to see that.
It's a good idea but for this review it's no longer applicable because the logic has been moved to Master::visit().
I agree in the future we should strive to test individual methods and in fact refactor long methods in Master. But in this case it proved to be difficult.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49741
-----------------------------------------------------------
On Aug. 6, 2014, 4:13 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:13 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp a925a936bc1cd878f1c9ff63a9dcc4ee808ec7a5
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49741
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87047>
it seems to me this could have some unit tests created outside of master tests. I'd really like to see that.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87048>
Option<size_t> capacity;
if (limit_.has_capacity()) {
capacity = limit_.capacity();
}
should work due to implicit conversion and is much clearer.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87049>
see above
- Dominic Hamon
On Aug. 5, 2014, 10:53 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 5, 2014, 10:53 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 6, 2014, 11:30 a.m., Vinod Kone wrote:
> > src/tests/rate_limiting_tests.cpp, lines 973-975
> > <https://reviews.apache.org/r/24343/diff/2/?file=653522#file653522line973>
> >
> > Isn't this already done by StartMaster()?
Yes. I lost track of the change :)
> On Aug. 6, 2014, 11:30 a.m., Vinod Kone wrote:
> > src/master/master.hpp, line 792
> > <https://reviews.apache.org/r/24343/diff/2/?file=653520#file653520line792>
> >
> > seems weird for this to be a CHECK, since the users of this class has no idea about the restriction. How about returning an Error instead?
> >
> > Try<process::Event*> nextPermittedEvent() {}
> >
> > Also, s/nextPermittedEvent/nextEvent/ ?
Removed EventThrottler and made it a dumb struct. Put the logic inside visit() per discussion with Vinod.
> On Aug. 6, 2014, 11:30 a.m., Vinod Kone wrote:
> > src/master/master.cpp, line 984
> > <https://reviews.apache.org/r/24343/diff/2/?file=653521#file653521line984>
> >
> > +1. This is not really a continuation i.e., never called asynchronously.
In master we already use continuations that are not called asynchronously e.g. __reregisterSlave(), or maybe I don't need to say it's a "continuation".
Anyway, __visit() is gone and the behavior the other _visit()s has changed in this new revision, please take a look.
> On Aug. 6, 2014, 11:30 a.m., Vinod Kone wrote:
> > src/tests/rate_limiting_tests.cpp, line 1048
> > <https://reviews.apache.org/r/24343/diff/2/?file=653522#file653522line1048>
> >
> > s/reach/processed/ ?
I meant to comment on the capacity with "reach".
I rephrased it to be:
// Send two messages which will be queued up. This will reach but not
// exceed the capacity.
> On Aug. 6, 2014, 11:30 a.m., Vinod Kone wrote:
> > src/tests/rate_limiting_tests.cpp, line 1071
> > <https://reviews.apache.org/r/24343/diff/2/?file=653522#file653522line1071>
> >
> > how are you making sure the deactivate message is received by the master?
DeactivateFrameworkMessage is sent by the driver upon receiving the error message so with Clock::settle() we can be sure the master has received it (not necessarily processed).
Should there be more comment to explain this?
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49761
-----------------------------------------------------------
On Aug. 6, 2014, 4:13 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:13 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 628cce12d2fae645d2ef55e4809631ca03a56207
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp a925a936bc1cd878f1c9ff63a9dcc4ee808ec7a5
> src/tests/cluster.hpp d857fc604f5b47cf9805b37c13051093f486ba31
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49761
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/24343/#comment87066>
s/are//
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87070>
s/head/the head/?
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment87071>
seems weird for this to be a CHECK, since the users of this class has no idea about the restriction. How about returning an Error instead?
Try<process::Event*> nextPermittedEvent() {}
Also, s/nextPermittedEvent/nextEvent/ ?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87073>
s/throttle_/throttle/ ?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87078>
+1. This is not really a continuation i.e., never called asynchronously.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment87081>
s/EventThrottler returns error://
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/24343/#comment87084>
Isn't this already done by StartMaster()?
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/24343/#comment87086>
s/reach/processed/ ?
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/24343/#comment87087>
s/it//
src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/24343/#comment87090>
how are you making sure the deactivate message is received by the master?
- Vinod Kone
On Aug. 6, 2014, 5:53 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 5:53 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/
-----------------------------------------------------------
(Updated Aug. 5, 2014, 10:53 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Dominic's comments.
Bugs: MESOS-1578
https://issues.apache.org/jira/browse/MESOS-1578
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
Diff: https://reviews.apache.org/r/24343/diff/
Testing
-------
make check
./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
Thanks,
Jiang Yan Xu
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.hpp, line 753
> > <https://reviews.apache.org/r/24343/diff/1/?file=653285#file653285line753>
> >
> > is there a reason this is a nested class in Master?
It only intended to be used in Master but I guess it doesn't need to be defined in Master.
Existing "internal" classes and structs defined in master.hpp are defined both inside and outside but the ones outside are accessed from http.cpp as well so I figured that's the reason.
In this case I just thought it logically belongs "inside". Any reason not to be put inside as nested?
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.hpp, line 766
> > <https://reviews.apache.org/r/24343/diff/1/?file=653285#file653285line766>
> >
> > might be worth adding the capacity to the error message to make it easier to debug issues.
Done.
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 939
> > <https://reviews.apache.org/r/24343/diff/1/?file=653286#file653286line939>
> >
> > maybe a const Option<string>& here?
Thanks!
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 377
> > <https://reviews.apache.org/r/24343/diff/1/?file=653286#file653286line377>
> >
> > please pull these out into more traditional if statements above the 'put' call. this is completely unreadable.
Done.
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 884
> > <https://reviews.apache.org/r/24343/diff/1/?file=653286#file653286line884>
> >
> > assuming there might be more errors in the future, this comment might be misleading.
Removed the comment and now pass the returned error to __visit() for better logging and error message sent to framework.
> On Aug. 5, 2014, 4:50 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 997
> > <https://reviews.apache.org/r/24343/diff/1/?file=653286#file653286line997>
> >
> > consider including the number of messages and principal.
Included the "number of messages", i.e., the capacity. Did not include the principal as I think it should be very clear to the framework what its own principal is (if it sets it in FrameworkInfo) and it could be empty (if it doesn't set it). "capacity" on the other hand is configured on the master so I think it's useful information.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49664
-----------------------------------------------------------
On Aug. 5, 2014, 10:53 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 5, 2014, 10:53 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 24343: Improved framework rate limiting by imposing the
max number of outstanding messages per framework principal.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24343/#review49664
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment86920>
is there a reason this is a nested class in Master?
src/master/master.hpp
<https://reviews.apache.org/r/24343/#comment86921>
might be worth adding the capacity to the error message to make it easier to debug issues.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment86922>
please pull these out into more traditional if statements above the 'put' call. this is completely unreadable.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment86923>
assuming there might be more errors in the future, this comment might be misleading.
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment86924>
maybe a const Option<string>& here?
src/master/master.cpp
<https://reviews.apache.org/r/24343/#comment86925>
consider including the number of messages and principal.
- Dominic Hamon
On Aug. 5, 2014, 4:41 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24343/
> -----------------------------------------------------------
>
> (Updated Aug. 5, 2014, 4:41 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1578
> https://issues.apache.org/jira/browse/MESOS-1578
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 6d4fd145004a14514e27ba65b2eae19b945aeb83
> src/examples/load_generator_framework.cpp 7d94c49cf91bf327ac80f04d9f1a7370996b6ba4
> src/master/master.hpp d8a4d9e04ecff60020b99ea6447055787d187797
> src/master/master.cpp 9bede96083f56768a5d42baf051af97de4c48d48
> src/tests/rate_limiting_tests.cpp fc23a1946ad1a78e699552440df2193ea10dc472
>
> Diff: https://reviews.apache.org/r/24343/diff/
>
>
> Testing
> -------
>
> make check
>
> ./bin/mesos-tests.sh --verbose --gtest_filter=*RateLimit* --gtest_repeat=1000
>
>
> Thanks,
>
> Jiang Yan Xu
>
>