> On March 6, 2014, 3:41 a.m., Ben Mahler wrote:
> > src/log/catchup.cpp, line 205
> > <https://reviews.apache.org/r/18368/diff/5/?file=511176#file511176line205>
> >
> >     If lower == upper, doesn't seem like we should be catching up the lower 
> > position!

Yeah, but I prefer to having only one termination check in the beginning of 
catchup(). Feel free to re-open if you are not OK with this.


> On March 6, 2014, 3:41 a.m., Ben Mahler wrote:
> > src/log/catchup.cpp, lines 331-338
> > <https://reviews.apache.org/r/18368/diff/5/?file=511176#file511176line331>
> >
> >     Oh.. ok I see why we should consider renaming these catchup functions, 
> > feel free to do so if you wan to clean this up.

Hum, now I prefer using explicit disambiguation as that's what we did in many 
places in our code base.


> On March 6, 2014, 3:41 a.m., Ben Mahler wrote:
> > src/log/replica.cpp, line 93
> > <https://reviews.apache.org/r/18368/diff/5/?file=511180#file511180line93>
> >
> >     Do you want this TODO in the header?

Fixed. Moved to cpp.


> On March 6, 2014, 3:41 a.m., Ben Mahler wrote:
> > src/log/replica.cpp, lines 269-270
> > <https://reviews.apache.org/r/18368/diff/5/?file=511180#file511180line269>
> >
> >     I don't think you need this comment, how about something more succinct?
> >     
> >     if (from > to) {
> >       // Empty interval.
> >       return ...;
> >     }

Done.


- Jie


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


On March 5, 2014, 8:15 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18368/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 8:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/log/catchup.hpp 45dc016 
>   src/log/catchup.cpp dae4619 
>   src/log/coordinator.cpp 6bfff1e 
>   src/log/recover.cpp 3403b47 
>   src/log/replica.hpp 08ddcb1 
>   src/log/replica.cpp 1f1a945 
>   src/log/storage.hpp c0eba1b 
>   src/tests/log_tests.cpp 7f2a740 
> 
> Diff: https://reviews.apache.org/r/18368/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to