> On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java, > > line 199 > > <https://reviews.apache.org/r/17895/diff/1/?file=481708#file481708line199> > > > > 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...
modified > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBaseV3.java, > > line 1 > > <https://reviews.apache.org/r/17895/diff/1/?file=481715#file481715line1> > > > > license header is missing no ? added > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java, > > line 498 > > <https://reviews.apache.org/r/17895/diff/1/?file=481716#file481716line498> > > > > Can we avoid String concatenation in logs ? done > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java, > > line 726 > > <https://reviews.apache.org/r/17895/diff/1/?file=481716#file481716line726> > > > > Avoid string concatenation in logs, have seen many such things in the > > patch. Could you please take care this. done > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java, > > line 447 > > <https://reviews.apache.org/r/17895/diff/1/?file=481716#file481716line447> > > > > Can we avoid String concatenation in logs ? since this task is for protocol change. so I just left the logs as unchanged as possible. but anyway, changed to {} in new patch. > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java, > > line 764 > > <https://reviews.apache.org/r/17895/diff/1/?file=481716#file481716line764> > > > > Avoid String concatenation in logs. done > On March 24, 2014, 11:55 a.m., Rakesh R wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java, > > line 1 > > <https://reviews.apache.org/r/17895/diff/1/?file=481720#file481720line1> > > > > license header is missing. done - Sijie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17895/#review38282 ----------------------------------------------------------- On April 21, 2014, 5:58 a.m., Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17895/ > ----------------------------------------------------------- > > (Updated April 21, 2014, 5:58 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 > 28e23d6 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > fb36b90 > > 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 > a10f7d5 > > 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 97a6156 > > 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 > >
