-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/
-----------------------------------------------------------

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

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review27
-----------------------------------------------------------

src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
<http://review.hbase.org/r/75/#comment178>

    2 spaces not 1

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment179>

    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.

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment180>

    remove this pointless catch, it just muddles the stack trace

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment181>

    ditto, remove the try/catch and let the exception bubble up
- Ryan
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
>
>

> 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
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review34
-----------------------------------------------------------

src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
<http://review.hbase.org/r/75/#comment203>

    There are 2 spaces before `throws', need just 1.
- Benoit
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
>
>

> On 2010-05-22 21:00:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java, line 75
> > <http://review.hbase.org/r/75/diff/1/?file=513#file513line75>
> >
> >     There are 2 spaces before `throws', need just 1.

Just testing review board.  Follow up test.
- Benoit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review34
-----------------------------------------------------------
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
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review36
-----------------------------------------------------------
One more test.
Also I hope we're going to fix the whole situation with IOExceptions all over the place.  They're defeating the whole purpose of checked exceptions are *really* annoying in client code (code that uses HBase).

- Benoit
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
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review30
-----------------------------------------------------------

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment190>

    i didnt note that, ok then lets keep it
- Ryan
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
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review37
-----------------------------------------------------------

Ship it!
- Ryan
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
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review38
-----------------------------------------------------------

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment205>

    Why not remove the try/catch and just let the IOE out?

src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/75/#comment206>

    Same here (Remove the try/catch and just let the IOE out)
- stack
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
>
>

> On 2010-05-22 22:38:21, stack wrote:
> >

This patch is radical.  Its going to throw up some new stuff.  Best to get it in now.  I had two minor issues.  I can fix on commit.
- stack
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review38
-----------------------------------------------------------
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
>
>

The try/catches are staying in to add the HFile.toString() to the
exception, so the user will have some indication of which file threw
the exception.

On Sat, May 22, 2010 at 10:39 PM,  <[EMAIL PROTECTED]> wrote:
>
>
>> On 2010-05-22 22:38:21, stack wrote:
>> >
>
> This patch is radical.  Its going to throw up some new stuff.  Best to get it in now.  I had two minor issues.  I can fix on commit.
>
>
> - stack
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/75/#review38
> -----------------------------------------------------------
>
>
> 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
>>
>>
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/75/#review43
-----------------------------------------------------------

Ship it!
I'm going to commit this.

- stack
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
>
>