> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, lines 92-94
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line92>
> >
> >     Using the word "recover(y)" in the description may help people 
> > understand the flag name "recovery_***"
> >     
> >     Maybe
> >     
> >     "The timeout within which all slaves are expected to re-register (be 
> > recovered) ..."?

removed "recovery_" prefix per benh's comment below.


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/main.cpp, line 185
> > <https://reviews.apache.org/r/20572/diff/1/?file=564784#file564784line185>
> >
> >     alignment off by 1 space.

good catch!


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/constants.hpp, lines 56-59
> > <https://reviews.apache.org/r/20572/diff/1/?file=564781#file564781line56>
> >
> >     I know it's the current way we do things but for the things we put in 
> > flags why do we put their defaults in a separate constants file instead of 
> > just defining them within flags.hpp?
> >     It'll be confusing whether to use the flag or the constants especially 
> > when the variables don't say "DEFAULT_***" Plus there is the non-POD global 
> > variable problem.

I think this is partly because of the gcc bug mentioned in constants.hpp. We 
would need a master/flags.cpp if we want to put these flag constants in 
master/flags.hpp. I will punt on this for now but will add a TODO.


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, line 90
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line90>
> >
> >     I guess slave backoff can't really use this because it doesn't handle 
> > "failover recovery" separately and still need to reregister within 75secs 
> > in case it's a network/ZK blip.

if it's a ZK blip only at the slave, the master wouldn't realize the slave 
disconnection. so the slave can always bound its re-registration retries on 
this value irrespective of whether the master failed over or not. does that 
make sense?


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41080
-----------------------------------------------------------


On April 22, 2014, 8:02 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 8:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Tests upcoming. I could either do it in this review or a subsequent one if 
> others want to pitch in.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to