> 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
> 
>

Reply via email to