----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1115/#review1700 -----------------------------------------------------------
This looks great to me. Only one thing that seems odd, the first thing below. Nicolas, what you think? src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java <http://review.cloudera.org/r/1115/#comment5614> Do you need to do this? You can't get your WALReader in before first construction? I think I see why you want to use it... its because the test already has an HLog? What about just making a new HLog instance in your tests so you don't have to have this? src/test/java/org/apache/hadoop/hbase/regionserver/wal/FaultySequenceFileLogReader.java <http://review.cloudera.org/r/1115/#comment5615> enum.valueOf might help here? see http://leepoint.net/notes-java/oop/enums/enums.html - stack On 2010-10-28 17:03:17, Alex Newman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1115/ > ----------------------------------------------------------- > > (Updated 2010-10-28 17:03:17) > > > Review request for hbase. > > > Summary > ------- > > So I took the approach taken with the instrumented sequence file rider. Not > sure if exposing those methods in the WALReader is cool so I am down with > other suggestions. > > - Add a FaultySequenceFileLogReader > - Un-commented the 2935 tests > > > This addresses bug hbase-2935. > http://issues.apache.org/jira/browse/hbase-2935 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java c8a10af > > src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java > 0b7dc78 > > src/test/java/org/apache/hadoop/hbase/regionserver/wal/FaultySequenceFileLogReader.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java > 473c359 > > Diff: http://review.cloudera.org/r/1115/diff > > > Testing > ------- > > Running on our internal hudson as I type this. Also I ran the TestHLogSplit > test locally. I'll update the review if hudson is green. > > > Thanks, > > Alex > >
