commit 3149687526c7ebdbc53a437b3257bd98e6b01616 Author: Todd Lipcon Date: Fri May 7 01:04:48 2010 -0700 HBASE-2519. IOExceptions scanning HFiles should bubble all the way through to client diff --git src/java/org/apache/hadoop/hbase/client/ScannerCallable.java src/java/org/apache/hadoop/hbase/client/ScannerCallable.java index 4b99052..95fa3a5 100644 --- src/java/org/apache/hadoop/hbase/client/ScannerCallable.java +++ src/java/org/apache/hadoop/hbase/client/ScannerCallable.java @@ -81,15 +81,15 @@ public class ScannerCallable extends ServerCallable { if (e instanceof RemoteException) { ioe = RemoteExceptionHandler.decodeRemoteException((RemoteException)e); } - if (ioe != null) { - if (ioe instanceof NotServingRegionException) { + if (ioe == null) throw new IOException(e); + if (ioe instanceof NotServingRegionException) { // Throw a DNRE so that we break out of cycle of calling NSRE // when what we need is to open scanner against new location. // Attach NSRE to signal client that it needs to resetup scanner. throw new DoNotRetryIOException("Reset scanner", ioe); - } else if (ioe instanceof DoNotRetryIOException) { - throw ioe; - } + } else { + // The outer layers will retry + throw ioe; } } return rrs; diff --git src/java/org/apache/hadoop/hbase/regionserver/HRegion.java src/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 53566f9..b920ddd 100644 --- src/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ src/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -1946,7 +1946,7 @@ public class HRegion implements HConstants, HeapSize { // , Writable{ this(scan, null); } - void initHeap() { + void initHeap() throws IOException { List scanners = new ArrayList(); if (extraScanners != null) { scanners.addAll(extraScanners); diff --git src/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java src/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java index 97948e3..3f81f62 100644 --- src/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java +++ src/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java @@ -72,7 +72,7 @@ public class KeyValueHeap implements KeyValueScanner, InternalScanner { return this.current.peek(); } - public KeyValue next() { + public KeyValue next() throws IOException { if(this.current == null) { return null; } @@ -178,8 +178,9 @@ public class KeyValueHeap implements KeyValueScanner, InternalScanner { * automatically closed and removed from the heap. * @param seekKey KeyValue to seek at or after * @return true if KeyValues exist at or after specified key, false if not + * @throws IOException */ - public boolean seek(KeyValue seekKey) { + public boolean seek(KeyValue seekKey) throws IOException { if(this.current == null) { return false; } diff --git src/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java src/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java index 4c5b844..dc454d7 100644 --- src/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java +++ src/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java @@ -19,6 +19,8 @@ */ package org.apache.hadoop.hbase.regionserver; +import java.io.IOException; + import org.apache.hadoop.hbase.KeyValue; /** @@ -35,14 +37,14 @@ public interface KeyValueScanner { * Return the next KeyValue in this scanner, iterating the scanner * @return the next KeyValue */ - public KeyValue next(); + public KeyValue next() throws IOException; /** * Seek the scanner at or after the specified KeyValue. * @param key * @return true if scanner has values left, false if end of scanner */ - public boolean seek(KeyValue key); + public boolean seek(KeyValue key) throws IOException; /** * Close the KeyValue scanner. diff --git src/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java src/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java index 651f2a7..e352901 100644 --- src/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java +++ src/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java @@ -38,7 +38,7 @@ public class MinorCompactingStoreScanner implements KeyValueScanner, InternalSca private KeyValue.KVComparator comparator; MinorCompactingStoreScanner(Store store, - List scanners) { + List scanners) throws IOException { comparator = store.comparator; KeyValue firstKv = KeyValue.createFirstOnRow(HConstants.EMPTY_START_ROW); for (KeyValueScanner scanner : scanners ) { @@ -49,7 +49,7 @@ public class MinorCompactingStoreScanner implements KeyValueScanner, InternalSca } MinorCompactingStoreScanner(String cfName, KeyValue.KVComparator comparator, - List scanners) { + List scanners) throws IOException { this.comparator = comparator; KeyValue firstKv = KeyValue.createFirstOnRow(HConstants.EMPTY_START_ROW); @@ -64,7 +64,7 @@ public class MinorCompactingStoreScanner implements KeyValueScanner, InternalSca return heap.peek(); } - public KeyValue next() { + public KeyValue next() throws IOException { return heap.next(); } diff --git src/java/org/apache/hadoop/hbase/regionserver/Store.java src/java/org/apache/hadoop/hbase/regionserver/Store.java index 77ea491..6b0e272 100644 --- src/java/org/apache/hadoop/hbase/regionserver/Store.java +++ src/java/org/apache/hadoop/hbase/regionserver/Store.java @@ -1301,9 +1301,10 @@ public class Store implements HConstants, HeapSize { /** * Return a scanner for both the memstore and the HStore files + * @throws IOException */ protected KeyValueScanner getScanner(Scan scan, - final NavigableSet targetCols) { + final NavigableSet targetCols) throws IOException { lock.readLock().lock(); try { return new StoreScanner(this, scan, targetCols); diff --git src/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java src/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java index ab51986..95e6844 100644 --- src/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java +++ src/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java @@ -79,7 +79,7 @@ class StoreFileScanner implements KeyValueScanner { return cur; } - public KeyValue next() { + public KeyValue next() throws IOException { KeyValue retKey = cur; cur = hfs.getKeyValue(); try { @@ -88,12 +88,12 @@ class StoreFileScanner implements KeyValueScanner { hfs.next(); } catch(IOException e) { // Turn checked exception into runtime exception. - throw new RuntimeException(e); + throw new IOException("Could not iterate " + this, e); } return retKey; } - public boolean seek(KeyValue key) { + public boolean seek(KeyValue key) throws IOException { try { if(!seekAtOrAfter(hfs, key)) { close(); @@ -103,9 +103,8 @@ class StoreFileScanner implements KeyValueScanner { hfs.next(); return true; } catch(IOException ioe) { - LOG.error("Could not seek in store file scanner " + this, ioe); - close(); - return false; + // Catch and rethrow so the IOE contains the path + throw new IOException("Could not seek " + this, ioe); } } diff --git src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java index adbcc9e..c38be14 100644 --- src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java +++ src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java @@ -52,8 +52,9 @@ class StoreScanner implements KeyValueScanner, InternalScanner, ChangedReadersOb * @param store who we scan * @param scan the spec * @param columns which columns we are scanning + * @throws IOException */ - StoreScanner(Store store, Scan scan, final NavigableSet columns) { + StoreScanner(Store store, Scan scan, final NavigableSet columns) throws IOException { this.store = store; this.cacheBlocks = scan.getCacheBlocks(); matcher = new ScanQueryMatcher(scan, store.getFamily().getName(), @@ -82,8 +83,10 @@ class StoreScanner implements KeyValueScanner, InternalScanner, ChangedReadersOb * @param store who we scan * @param scan the spec * @param scanners ancilliary scanners + * @throws IOException */ - StoreScanner(Store store, Scan scan, List scanners) { + StoreScanner(Store store, Scan scan, List scanners) + throws IOException { this.store = store; this.cacheBlocks = false; this.isGet = false; @@ -104,7 +107,8 @@ class StoreScanner implements KeyValueScanner, InternalScanner, ChangedReadersOb StoreScanner(final Scan scan, final byte [] colFamily, final long ttl, final KeyValue.KVComparator comparator, final NavigableSet columns, - final List scanners) { + final List scanners) + throws IOException { this.store = null; this.isGet = false; this.cacheBlocks = scan.getCacheBlocks(); @@ -150,7 +154,7 @@ class StoreScanner implements KeyValueScanner, InternalScanner, ChangedReadersOb this.heap.close(); } - public synchronized boolean seek(KeyValue key) { + public synchronized boolean seek(KeyValue key) throws IOException { return this.heap.seek(key); } diff --git src/test/org/apache/hadoop/hbase/TestIOFencing.java src/test/org/apache/hadoop/hbase/TestIOFencing.java index d5dab96..fe05fc8 100644 --- src/test/org/apache/hadoop/hbase/TestIOFencing.java +++ src/test/org/apache/hadoop/hbase/TestIOFencing.java @@ -223,6 +223,10 @@ public class TestIOFencing { System.currentTimeMillis() - startWaitTime < 30000); } + HBaseConfiguration newConf = new HBaseConfiguration(); + // Force low number of retries to fail fast if we fail + newConf.setInt("hbase.client.retries.number", 3); + table = new HTable(newConf, TABLE_NAME); assertEquals(FIRST_BATCH_COUNT + SECOND_BATCH_COUNT, TEST_UTIL.countRows(table)); } finally { diff --git src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java index 33933ad..757e370 100644 --- src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java +++ src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.regionserver; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -71,7 +72,7 @@ implements HConstants { col5 = Bytes.toBytes("col5"); } - public void testSorted(){ + public void testSorted() throws IOException{ //Cases that need to be checked are: //1. The "smallest" KeyValue is in the same scanners as current //2. Current scanner gets empty @@ -133,7 +134,7 @@ implements HConstants { } - public void testSeek(){ + public void testSeek() throws IOException{ //Cases: //1. Seek KeyValue that is not in scanner //2. Check that smallest that is returned from a seek is correct @@ -181,7 +182,7 @@ implements HConstants { } - public void testScannerLeak() { + public void testScannerLeak() throws IOException { // Test for unclosed scanners (HBASE-1927) List l1 = new ArrayList(); diff --git src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java index e1ffcc3..0ebeee4 100644 --- src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java +++ src/test/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java @@ -20,6 +20,8 @@ package org.apache.hadoop.hbase.regionserver; +import java.io.IOException; + import junit.framework.TestCase; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.KeyValueTestUtil; @@ -28,7 +30,7 @@ import org.apache.hadoop.hbase.util.Bytes; public class TestKeyValueScanFixture extends TestCase { - public void testKeyValueScanFixture() { + public void testKeyValueScanFixture() throws IOException { KeyValue kvs[] = new KeyValue[]{ KeyValueTestUtil.create("RowA", "family", "qf1", 1, KeyValue.Type.Put, "value-1"), diff --git src/test/org/apache/hadoop/hbase/regionserver/TestMemStore.java src/test/org/apache/hadoop/hbase/regionserver/TestMemStore.java index 464c1cf..0a9da81 100644 --- src/test/org/apache/hadoop/hbase/regionserver/TestMemStore.java +++ src/test/org/apache/hadoop/hbase/regionserver/TestMemStore.java @@ -157,8 +157,9 @@ public class TestMemStore extends TestCase { /** * A simple test which verifies the 3 possible states when scanning across snapshot. + * @throws IOException */ - public void testScanAcrossSnapshot2() { + public void testScanAcrossSnapshot2() throws IOException { // we are going to the scanning across snapshot with two kvs // kv1 should always be returned before kv2 final byte[] one = Bytes.toBytes(1); @@ -187,7 +188,8 @@ public class TestMemStore extends TestCase { verifyScanAcrossSnapshot2(kv1, kv2); } - private void verifyScanAcrossSnapshot2(KeyValue kv1, KeyValue kv2) { + private void verifyScanAcrossSnapshot2(KeyValue kv1, KeyValue kv2) + throws IOException { ReadWriteConsistencyControl.resetThreadReadPoint(rwcc); List memstorescanners = this.memstore.getScanners(); assertEquals(1, memstorescanners.size()); @@ -198,7 +200,8 @@ public class TestMemStore extends TestCase { assertNull(scanner.next()); } - private void assertScannerResults(KeyValueScanner scanner, KeyValue[] expected) { + private void assertScannerResults(KeyValueScanner scanner, KeyValue[] expected) + throws IOException { scanner.seek(KeyValue.createFirstOnRow(new byte[]{})); for (KeyValue kv : expected) { assertTrue(0 == @@ -208,7 +211,7 @@ public class TestMemStore extends TestCase { assertNull(scanner.peek()); } - public void testMemstoreConcurrentControl() { + public void testMemstoreConcurrentControl() throws IOException { final byte[] row = Bytes.toBytes(1); final byte[] f = Bytes.toBytes("family"); final byte[] q1 = Bytes.toBytes("q1"); @@ -249,7 +252,6 @@ public class TestMemStore extends TestCase { } private static class ReadOwnWritesTester extends Thread { - final int id; static final int NUM_TRIES = 1000; final byte[] row; @@ -268,7 +270,6 @@ public class TestMemStore extends TestCase { ReadWriteConsistencyControl rwcc, AtomicReference caughtException) { - this.id = id; this.rwcc = rwcc; this.memstore = memstore; this.caughtException = caughtException; @@ -283,7 +284,7 @@ public class TestMemStore extends TestCase { } } - private void internalRun() { + private void internalRun() throws IOException { for (long i = 0; i < NUM_TRIES && caughtException.get() == null; i++) { ReadWriteConsistencyControl.WriteEntry w = rwcc.beginMemstoreInsert(); @@ -854,7 +855,7 @@ public class TestMemStore extends TestCase { } - static void doScan(MemStore ms, int iteration) { + static void doScan(MemStore ms, int iteration) throws IOException { long nanos = System.nanoTime(); KeyValueScanner s = ms.getScanners().get(0); s.seek(KeyValue.createFirstOnRow(new byte[]{})); @@ -867,7 +868,7 @@ public class TestMemStore extends TestCase { } - public static void main(String [] args) { + public static void main(String [] args) throws IOException { ReadWriteConsistencyControl rwcc = new ReadWriteConsistencyControl(); MemStore ms = new MemStore();