> On March 5, 2014, 8:01 p.m., Benjamin Hindman wrote:
> > src/master/registrar.cpp, line 248
> > <https://reviews.apache.org/r/18675/diff/2/?file=509871#file509871line248>
> >
> >     Not that you need it, but this was lost in the refactor FYI.

Added this back!


> On March 5, 2014, 8:01 p.m., Benjamin Hindman wrote:
> > src/master/registrar.cpp, line 253
> > <https://reviews.apache.org/r/18675/diff/2/?file=509871#file509871line253>
> >
> >     Did you mean s/Recovery/Recover/ ?

Fixed.


> On March 5, 2014, 8:01 p.m., Benjamin Hindman wrote:
> > src/master/registrar.cpp, line 274
> > <https://reviews.apache.org/r/18675/diff/2/?file=509871#file509871line274>
> >
> >     Technically this is a 'failed to persist master info' case as well.

Fixed.


> On March 5, 2014, 8:01 p.m., Benjamin Hindman wrote:
> > src/tests/registrar_tests.cpp, line 81
> > <https://reviews.apache.org/r/18675/diff/2/?file=509872#file509872line81>
> >
> >     Here's a nudge to adding a test which does admit, readmit, or remove 
> > before recover and does an AWAIT_FAILED. ;)

Fixed.


> On March 5, 2014, 8:01 p.m., Benjamin Hindman wrote:
> > src/master/registrar.cpp, lines 277-280
> > <https://reviews.apache.org/r/18675/diff/2/?file=509871#file509871line277>
> >
> >     This is the hardest code to reason about because it requires non-local 
> > reasoning, i.e., the variable gets updated via the 'update' call in 
> > '_recover' and thus it is 'Some' here which is why you can set 'recovered' 
> > to it. Can you enhance this comment to make this flow more understandable 
> > to a future reader? Thanks!

Updated the comment here, in fact, 'variable' is some in _recover after we've 
fetched the Registry. At this point, we're relying on the update to 'variable' 
to be present from _update().


- Ben


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


On March 4, 2014, 8:05 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18675/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 8:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-764
>     https://issues.apache.org/jira/browse/MESOS-764
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As a safety measure, we would like to force a version change on the Registry 
> as a result of the recovery process. This helps prevent a "rogue" Master 
> (that still believes it is leading) from performing writes to the Registry.
> 
> This change adds a 'Recover' operation which adds the latest MasterInfo to 
> the Registry. Unfortunately, since the Registrar is injected to the Master, I 
> had to add 'MasterInfo' as an argument to the 'recover' function, thus 
> altering the design somewhat.
> 
> The current semantics are that there one must call 'recover' before calling 
> other operations, otherwise these other operations will fail.
> 
> 
> Diffs
> -----
> 
>   src/master/registrar.hpp 20734afc69055197e9ab90d42253c56e4af4b97c 
>   src/master/registrar.cpp 37337c07b24a96e71910b7c83085d159361a1188 
>   src/tests/registrar_tests.cpp 3bf42bd77a10470a2afc6fd8e1da30d6134e792c 
> 
> Diff: https://reviews.apache.org/r/18675/diff/
> 
> 
> Testing
> -------
> 
> Added a small test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to