From 63946bb8143af6416a1771060d6d11cddf1516b4 Mon Sep 17 00:00:00 2001 From: sunyerui Date: Sun, 27 Sep 2015 18:06:03 +0800 Subject: [PATCH] HBASE-14497 Reverse Scan threw StackOverflow caused by readPt checking --- .../hbase/regionserver/StoreFileScanner.java | 76 ++++++++++++---------- .../hadoop/hbase/regionserver/TestHRegion.java | 40 ++++++++++++ 2 files changed, 82 insertions(+), 34 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java index d9e2eab..7575d37 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java @@ -435,48 +435,56 @@ public class StoreFileScanner implements KeyValueScanner { } @Override - public boolean seekToPreviousRow(KeyValue key) throws IOException { + public boolean seekToPreviousRow(KeyValue originalKey) throws IOException { try { try { - KeyValue seekKey = KeyValue.createFirstOnRow(key.getRow()); - if (seekCount != null) seekCount.incrementAndGet(); - if (!hfs.seekBefore(seekKey.getBuffer(), seekKey.getKeyOffset(), - seekKey.getKeyLength())) { - close(); - return false; - } - KeyValue firstKeyOfPreviousRow = KeyValue.createFirstOnRow(hfs - .getKeyValue().getRow()); - - if (seekCount != null) seekCount.incrementAndGet(); - if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { - close(); - return false; - } - - cur = hfs.getKeyValue(); - this.stopSkippingKVsIfNextRow = true; - boolean resultOfSkipKVs; - try { - resultOfSkipKVs = skipKVsNewerThanReadpoint(); - } finally { - this.stopSkippingKVsIfNextRow = false; - } - if (!resultOfSkipKVs - || getComparator().compareRows(cur.getBuffer(), cur.getRowOffset(), - cur.getRowLength(), firstKeyOfPreviousRow.getBuffer(), - firstKeyOfPreviousRow.getRowOffset(), - firstKeyOfPreviousRow.getRowLength()) > 0) { - return seekToPreviousRow(firstKeyOfPreviousRow); - } - + boolean recursiveSeek = false; + KeyValue key = originalKey; + do { + KeyValue seekKey = KeyValue.createFirstOnRow(key.getRow()); + if (seekCount != null) seekCount.incrementAndGet(); + if (!hfs.seekBefore(seekKey.getBuffer(), seekKey.getKeyOffset(), + seekKey.getKeyLength())) { + close(); + return false; + } + KeyValue firstKeyOfPreviousRow = KeyValue.createFirstOnRow(hfs + .getKeyValue().getRow()); + + if (seekCount != null) seekCount.incrementAndGet(); + if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { + close(); + return false; + } + + cur = hfs.getKeyValue(); + this.stopSkippingKVsIfNextRow = true; + boolean resultOfSkipKVs; + try { + resultOfSkipKVs = skipKVsNewerThanReadpoint(); + } finally { + this.stopSkippingKVsIfNextRow = false; + } + if (!resultOfSkipKVs + || getComparator().compareRows(cur.getBuffer(), cur.getRowOffset(), + cur.getRowLength(), firstKeyOfPreviousRow.getBuffer(), + firstKeyOfPreviousRow.getRowOffset(), + firstKeyOfPreviousRow.getRowLength()) > 0) { + recursiveSeek = true; + key = firstKeyOfPreviousRow; + LOG.warn("seekToPreviousRow need to recursive seek to key " + key); + continue; + } else { + break; + } + } while (recursiveSeek); return true; } finally { realSeekDone = true; } } catch (IOException ioe) { throw new IOException("Could not seekToPreviousRow " + this + " to key " - + key, ioe); + + originalKey, ioe); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 3ace9a4..c6067fc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -5117,6 +5117,46 @@ public class TestHRegion { } } + @Test (timeout = 60000) + public void testReverseScannerStackOverflow() throws IOException { + byte[] cf1 = Bytes.toBytes("CF1"); + byte[][] families = {cf1}; + byte[] col = Bytes.toBytes("C"); + String method = this.getName(); + HBaseConfiguration conf = new HBaseConfiguration(); + // disable compactions in this test. + conf.setInt("hbase.hstore.compactionThreshold", 10000); + this.region = initHRegion(tableName, method, conf, families); + + Put put = new Put(Bytes.toBytes("19998")); + put.add(cf1, col, Bytes.toBytes("val")); + Put put2 = new Put(Bytes.toBytes("09999")); + put2.add(cf1, col, Bytes.toBytes("val")); + region.put(put); + region.put(put2); + region.flushcache(); + + Scan scan = new Scan(Bytes.toBytes("19998")); + scan.setReversed(true); + InternalScanner scanner = region.getScanner(scan); + + for (int i = 10000; i < 20000; i++) { + Put p = new Put(Bytes.toBytes(""+i)); + p.add(cf1, col, Bytes.toBytes(""+i)); + region.put(p); + } + region.flushcache(); + + List currRow = new ArrayList(); + boolean hasNext; + do { + hasNext = scanner.next(currRow); + } while (hasNext); + assertEquals(2, currRow.size()); + assertArrayEquals(Bytes.toBytes("19998"), currRow.get(0).getRow()); + assertArrayEquals(Bytes.toBytes("09999"), currRow.get(1).getRow()); + } + @Test (timeout=60000) public void testSplitRegionWithReverseScan() throws IOException { byte [] tableName = Bytes.toBytes("testSplitRegionWithReverseScan"); -- 2.3.2 (Apple Git-55)