> On 2012-03-20 01:27:50, Todd Lipcon wrote: > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlockReaderLocal.java, > > line 44 > > <https://reviews.apache.org/r/4212/diff/2/?file=90213#file90213line44> > > > > shouldn't this be true?
Oops, yes. Thankfully the test still passes when it's testing the right path... > On 2012-03-20 01:27:50, Todd Lipcon wrote: > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlockReaderLocal.java, > > lines 81-82 > > <https://reviews.apache.org/r/4212/diff/2/?file=90213#file90213line81> > > > > no reason to use DFSClient here. Instead you can just use the > > filesystem, right? Then downcast the stream you get back? Good point - no need even to downcast since FSDataInputStream has the API. > On 2012-03-20 01:27:50, Todd Lipcon wrote: > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlockReaderLocal.java, > > line 104 > > <https://reviews.apache.org/r/4212/diff/2/?file=90213#file90213line104> > > > > don't you want an assert on sawException here? You can also use > > GenericTestUtils.assertExceptionContains() if you want to check the text of > > it Good catch. No particular need to assert the content of the exception - any checksum error is good enough here. > On 2012-03-20 01:27:50, Todd Lipcon wrote: > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderLocal.java, > > lines 562-564 > > <https://reviews.apache.org/r/4212/diff/2/?file=90207#file90207line562> > > > > this comment seems like it's in the wrong spot, since the code that > > comes after it doesn't reference offsetFromChunkBoundary. I removed the comment, it's covered by the comment at line 549. - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4212/#review6103 ----------------------------------------------------------- On 2012-03-09 00:47:24, Henry Robinson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4212/ > ----------------------------------------------------------- > > (Updated 2012-03-09 00:47:24) > > > Review request for hadoop-hdfs and Todd Lipcon. > > > Summary > ------- > > New patch for HDFS-2834 (I can't update the old review request). > > > This addresses bug HDFS-2834. > http://issues.apache.org/jira/browse/HDFS-2834 > > > Diffs > ----- > > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReader.java > dfab730 > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderLocal.java > cc61697 > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java > 4187f1c > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSInputStream.java > 2b817ff > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/RemoteBlockReader.java > b7da8d4 > > hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/RemoteBlockReader2.java > ea24777 > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/BlockReaderTestUtil.java > 9d4f4a2 > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestBlockReaderLocal.java > PRE-CREATION > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestParallelRead.java > bbd0012 > > hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestShortCircuitLocalRead.java > eb2a1d8 > > Diff: https://reviews.apache.org/r/4212/diff > > > Testing > ------- > > > Thanks, > > Henry > >
