----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18368/#review36389 -----------------------------------------------------------
Ship it! src/log/catchup.cpp <https://reviews.apache.org/r/18368/#comment67326> Failure is definitely better than the CHECK! Alternatively, if catchup() did a '>=' in the if condition then we would handle the empty interval as being caught up immediately rather than via a Failure, right? It would be nice if this did the right thing for an empty interval, now that we're passing an 'Interval' object. Perhaps you can move the empty interval check up to the appropriate level in the code? If an 'IntervalSet' cannot contain an empty 'Interval' is the assumption here, how about adding a CHECK where we iterate over the 'IntervalSet' if you're concerned about that case? - Ben Mahler On March 6, 2014, 6:21 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18368/ > ----------------------------------------------------------- > > (Updated March 6, 2014, 6:21 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 > >
