----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3737/#review5029 -----------------------------------------------------------
ah, thanks Ivan for fixing the wrong assignment of ledger length. the new patch is good to me. but I don't like the new callback name 'RecoveredDataCallback', which sounds like that the data is returned by some recovery actions although it doesn't. This data is retrieved by reading last confirmed without recovery. I prefer changing this callback to 'ReadLastConfirmedDataCallback' and the method to 'readLastConfirmedDataComplete'. - Sijie On 2012-02-10 15:23:20, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3737/ > ----------------------------------------------------------- > > (Updated 2012-02-10 15:23:20) > > > Review request for bookkeeper. > > > Summary > ------- > > Proposed fix ensures that at least one of each quorum replies to > ReadLastConfirmed. > > Refactors code a bit to make the read last confirmed common for recovery and > standalone read last confirmed. > > The bug here was actually that we were waiting for quorumSize responses, from > the bookies, when really all we need to get a response from one bookie in > each possible quorum. in the 2/2 case as above this means only 1 bookie need > response. > > There's a fix for the timeouts and an improvement in fencing which fixing > this uncovered. > > > This addresses bug BOOKKEEPER-152. > https://issues.apache.org/jira/browse/BOOKKEEPER-152 > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java > ae375ec > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java > f2ed6bd > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java > e3d1847 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > 4625bbb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java > 43e999d > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java > 4a88747 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java > a68fc8c > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > cbd2277 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java > da52ca5 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java > 5873255 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java > 77a2f69 > > Diff: https://reviews.apache.org/r/3737/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
