----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3737/#review5006 -----------------------------------------------------------
most is good to me. but it seems that you didn't assign right last confirmed value in readLastConfirmedOp callback. bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java <https://reviews.apache.org/r/3737/#comment11009> actually the result is not maxAddConfirmed, is lastAddConfirmed. maxAddConfirmed is member parameter in LedgerRecoveryOp. - Sijie On 2012-02-10 09:47:35, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3737/ > ----------------------------------------------------------- > > (Updated 2012-02-10 09:47:35) > > > 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/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 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java > 4a88747 > > 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 > > Diff: https://reviews.apache.org/r/3737/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
