mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 18368: Updated log to use the new interval set abstraction.
Date Wed, 05 Mar 2014 20:07:53 GMT


> On March 5, 2014, 12:54 a.m., Ben Mahler wrote:
> > src/log/replica.cpp, lines 268-269
> > <https://reviews.apache.org/r/18368/diff/3/?file=507720#file507720line268>
> >
> >     If this is really an error, why are we returning an empty interval set?
> 
> Jie Yu wrote:
>     Well, this matches the previous semantics. I don't wanna introduce a new semantics
here.
> 
> Jie Yu wrote:
>     Well, this matches the previous semantics. I don't wanna introduce a new semantics
here. Added a TODO to clean this up later.
> 
> Ben Mahler wrote:
>     What I mean is: is this really an error or is the definition of 'missing' the empty
set when from > to? The fact that you added a LOG(ERROR) makes me think it is the former.
> 
> Ben Mahler wrote:
>     Chatted with Jie about what to do here.
>     
>     Since the definition of 'missing' for the empty interval is the empty set, we should
just do the right thing and return the empty set without reporting an internal error. Mathematically,
(and this is how boost's interval works), 'from' > 'to' represents the empty interval.
Ideally we can design our API here to push the 'Interval' semantics to the callers, for example,
Replica::missing can take an 'Interval':
>     
>     Future<IntervalSet> ReplicaProcess::missing(const Interval& interval)
>     {
>       // Here we simply return the set of positions in the interval that are "missing",
>       // when the empty interval is passed, the empty set is returned.
>     }
>     
>     Technically, 'Replica::read' has similar semantics where we return a Failure instead
of the empty list, so it would be interesting to know why Failure is returned there.

Fixed. Added a comment and a TODO.

> Technically, 'Replica::read' has similar semantics where we return a Failure instead
of the empty list, so it would > be interesting to know why Failure is returned there.

IMO, 'missing' is a check and 'read' is an action. We should not return a failure for a 'check',
but we should return failure for an invalid 'action'.


- Jie


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


On March 5, 2014, 2:14 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18368/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 2:14 a.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message