----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3642/#review4668 -----------------------------------------------------------
If I understand the overall flow right, giving the wrong password will make readEntryAndDoFencing fail in the initiate call of the recovery procedure. It fails because the bookie server checks and returns an error (IOException in BookieServer). I don't fully understand the flow once we determine that the password is incorrect based on the two comments below. bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java <https://reviews.apache.org/r/3642/#comment10387> Is this where we fail the openLedger? If so, by checking the method, it doesn't seem to be using the right code. This is related to my other comment on the exception not being the right one. bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java <https://reviews.apache.org/r/3642/#comment10385> I don't think we currently throw this exception anywhere in the code (BKLedgerRecoveryException). It is nowhere else in this patch and it is not present in trunk. I think we are throwing an exception here, but not the the right one. - fpj On 2012-01-26 19:26:03, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3642/ > ----------------------------------------------------------- > > (Updated 2012-01-26 19:26:03) > > > Review request for bookkeeper. > > > Summary > ------- > > When fencing, the ledger handle is not checked before the fencing is applied. > Currently the openLedger does fail, on because it will addEntry and fail at > that point, but by this stage, fencing has already been applied. The check > should be earlier. > > > This addresses bug BOOKKEEPER-135. > https://issues.apache.org/jira/browse/BOOKKEEPER-135 > > > Diffs > ----- > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java > cb3bb26 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > 4625bbb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 29070eb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java > 43e999d > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java > 4466ce3 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java > bc1cfb0 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java > beab5e8 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java > 2fa79bb > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > 8526db5 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java > 015e4e4 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java > 99d6ef0 > > Diff: https://reviews.apache.org/r/3642/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
