----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6789/#review11536 -----------------------------------------------------------
the patch and the test looks good. I just had a minor comment and a question needs your clarification. hedwig-protocol/src/main/java/org/apache/hedwig/protoextensions/MessageIdUtils.java <https://reviews.apache.org/r/6789/#comment24803> could you describe more about this case? I am not clear about it. hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigHub.java <https://reviews.apache.org/r/6789/#comment24810> If this changes is only used for TestHubRegion, I would prefer putting this checking in TestHubRegion. you could inherit TestMessageHandler in TestHubRegion. - Sijie Guo On Aug. 27, 2012, 9:27 p.m., Aniruddha Laud wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6789/ > ----------------------------------------------------------- > > (Updated Aug. 27, 2012, 9:27 p.m.) > > > Review request for bookkeeper, Ivan Kelly, Ivan Kelly, and Sijie Guo. > > > Description > ------- > > https://issues.apache.org/jira/browse/BOOKKEEPER-256 > > > Diffs > ----- > > > hedwig-protocol/src/main/java/org/apache/hedwig/protoextensions/MessageIdUtils.java > 9ceec26 > hedwig-server/pom.xml 9f5c3cd > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/BookkeeperPersistenceManager.java > d2c4298 > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/LocalDBPersistenceManager.java > 16b7b7d > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/ReadAheadCache.java > 060a3dd > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/RegionManager.java > 838eca0 > > hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigHub.java > 02b4503 > > hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigRegion.java > 0b1851e > > Diff: https://reviews.apache.org/r/6789/diff/ > > > Testing > ------- > > mvn test passes. > > > Thanks, > > Aniruddha Laud > >
