----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3642/#review6519 -----------------------------------------------------------
the whole patch is very good. I just have some slight comments as below. bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java <https://reviews.apache.org/r/3642/#comment14182> just curious, do we need any cases to test different version compatibility? bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java <https://reviews.apache.org/r/3642/#comment14183> it would better to define 20 as constant, since there is several places referencing it. bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java <https://reviews.apache.org/r/3642/#comment14181> it would better to throw a BookieException, so it would be caught as EUA error. - Sijie On 2012-03-20 18:13:56, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3642/ > ----------------------------------------------------------- > > (Updated 2012-03-20 18:13:56) > > > 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 > ad41ba5 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java > 911c660 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java > 56186ab > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > c67a79c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java > 7aad751 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 539d6b2 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java > 7dd5363 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java > 8a32c64 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java > 8598c08 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java > 1a315e1 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java > 75a8e8c > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > b8923e8 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java > 7de1c10 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java > 99d6ef0 > > Diff: https://reviews.apache.org/r/3642/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
