----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17895/#review38282 -----------------------------------------------------------
Thanks Sijie, its really awsome! I just have few minor comments, please see it. bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java <https://reviews.apache.org/r/17895/#comment70327> Its good to modify the java comments as well as the logging which describes the new condition. Also to understand more about the issue, can you please point me to any test cases for these changes... bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBaseV3.java <https://reviews.apache.org/r/17895/#comment70328> license header is missing no ? bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java <https://reviews.apache.org/r/17895/#comment70331> Can we avoid String concatenation in logs ? bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java <https://reviews.apache.org/r/17895/#comment70332> Can we avoid String concatenation in logs ? bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java <https://reviews.apache.org/r/17895/#comment70333> Avoid string concatenation in logs, have seen many such things in the patch. Could you please take care this. bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java <https://reviews.apache.org/r/17895/#comment70330> Avoid String concatenation in logs. bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java <https://reviews.apache.org/r/17895/#comment70334> license header is missing. - Rakesh R On Feb. 10, 2014, 7:11 a.m., Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17895/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2014, 7:11 a.m.) > > > Review request for bookkeeper and Ivan Kelly. > > > Bugs: BOOKKEEPER-582 > https://issues.apache.org/jira/browse/BOOKKEEPER-582 > > > Repository: bookkeeper-git > > > Description > ------- > > - introducing protobuf support for bookkeeper > - for server: introduce packet processor / EnDecoder for different protocol > supports > - for client: change PCBC to use protobuf to send requests > - misc changes for protobuf support > > (bookie server is able for backward compatibility) > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java > 56487aa > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java > 962f3a3 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 241fdbb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/processor/RequestProcessor.java > 241f369 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java > 1154047 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestHandler.java > b922a82 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestProcessor.java > 8155b22 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookkeeperProtocol.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBaseV3.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java > 0757f87 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessor.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessorV3.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessor.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java > PRE-CREATION > bookkeeper-server/src/main/proto/BookkeeperProtocol.proto PRE-CREATION > bookkeeper-server/src/main/resources/findbugsExclude.xml 8404e3f > > bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestProtoVersions.java > 5fcc445 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorPeriodicCheckTest.java > 3f8496f > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java > bc05229 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestBackwardCompat.java > 8376b46 > > Diff: https://reviews.apache.org/r/17895/diff/ > > > Testing > ------- > > unit tests. backward tests. > > > Thanks, > > Sijie Guo > >
