> On 2010-05-21 15:53:11, Ryan Rawson wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java, > > line 63 > > <http://review.hbase.org/r/75/diff/1/?file=517#file517line63> > > > > why are we creating a reader here? The incoming store file list is > > already opened... Re-opening the file might not be the best choice, since > > if the on-disk situation becomes different from the in-memory we should > > probably be failing the compactions. Thus the code should be like what it > > was before only throwing an exception instead of continuing on.
createReader is lazy - if the file is already open it returns the existing one. I changed to create() since it was handy to use from the tests (and some other patches on top of this use it in that way as well) > On 2010-05-21 15:53:11, Ryan Rawson wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java, > > line 94 > > <http://review.hbase.org/r/75/diff/1/?file=517#file517line94> > > > > remove this pointless catch, it just muddles the stack trace The point of this is that you get the toString of the StoreFileScanner, which includes path name, etc. Otherwise you often get a less useful error like "Could not get block locations for blk_234234, aborting". So while the stack trace is definitely longer, I think there is actual value in having as much information here as possible. - Todd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/75/#review27 ----------------------------------------------------------- On 2010-05-21 15:23:07, Todd Lipcon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/75/ > ----------------------------------------------------------- > > (Updated 2010-05-21 15:23:07) > > > Review request for hbase and Ryan Rawson. > > > Summary > ------- > > In particular fixes issues where a compaction that got an error on one > storefile would happily proceed and just remove all that data. Or a user scan > would just show empty results instead of an error. > > > This addresses bug HBASE-2519. > http://issues.apache.org/jira/browse/HBASE-2519 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 70b85af > src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 3433811 > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 038a335 > src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java > 70f42dc > src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java > 657018f > > src/main/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java > 4b16540 > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 6c3153b > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java > 52d228b > src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java > fde872c > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 238e804 > src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 55a926f > src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java > 228ab2c > > src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java > e1ffcc3 > src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java > 6ed0209 > src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java > fd77329 > > Diff: http://review.hbase.org/r/75/diff > > > Testing > ------- > > new unit tests > > > Thanks, > > Todd > >
