> On Sept. 14, 2012, 2:06 p.m., Sijie Guo wrote: > > hedwig-server/src/test/java/org/apache/hedwig/server/integration/TestHedwigHub.java, > > line 159 > > <https://reviews.apache.org/r/6789/diff/1/?file=146653#file146653line159> > > > > If this changes is only used for TestHubRegion, I would prefer putting > > this checking in TestHubRegion. you could inherit TestMessageHandler in > > TestHubRegion.
It represents an invariant that should hold all the time though, so I don't think there's a problem keeping it here. It may flag bad changes in the future. > On Sept. 14, 2012, 2:06 p.m., Sijie Guo wrote: > > hedwig-protocol/src/main/java/org/apache/hedwig/protoextensions/MessageIdUtils.java, > > line 143 > > <https://reviews.apache.org/r/6789/diff/1/?file=146647#file146647line143> > > > > could you describe more about this case? I am not clear about it. I'm also unclear on this. In this else branch, you're basically copying all region specific seqid which are not from the src region. This is correct. I don't see where redelivery comes into it. - Ivan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6789/#review11536 ----------------------------------------------------------- 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 > >
