From ccce2d81f0d9a278c7ff78b017fffe46aca96efb Mon Sep 17 00:00:00 2001 From: sunyerui Date: Sun, 27 Sep 2015 18:30:19 +0800 Subject: [PATCH] HBASE-14497 Reverse Scan threw StackOverflow caused by readPt checking --- .../hbase/regionserver/StoreFileScanner.java | 72 ++++++++++++---------- .../hadoop/hbase/regionserver/TestHRegion.java | 40 ++++++++++++ 2 files changed, 80 insertions(+), 32 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 fde40e9..3a88ad0 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 @@ -437,39 +437,47 @@ public class StoreFileScanner implements KeyValueScanner { @Override @SuppressWarnings("deprecation") - public boolean seekToPreviousRow(Cell key) throws IOException { + public boolean seekToPreviousRow(Cell originalKey) throws IOException { try { try { - KeyValue seekKey = KeyValueUtil.createFirstOnRow(key.getRowArray(), key.getRowOffset(), - key.getRowLength()); - if (seekCount != null) seekCount.incrementAndGet(); - if (!hfs.seekBefore(seekKey.getBuffer(), seekKey.getKeyOffset(), - seekKey.getKeyLength())) { - close(); - return false; - } - KeyValue firstKeyOfPreviousRow = KeyValueUtil.createFirstOnRow(hfs.getKeyValue() - .getRowArray(), hfs.getKeyValue().getRowOffset(), hfs.getKeyValue().getRowLength()); - - if (seekCount != null) seekCount.incrementAndGet(); - if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { - close(); - return false; - } - - setCurrentCell(hfs.getKeyValue()); - this.stopSkippingKVsIfNextRow = true; - boolean resultOfSkipKVs; - try { - resultOfSkipKVs = skipKVsNewerThanReadpoint(); - } finally { - this.stopSkippingKVsIfNextRow = false; - } - if (!resultOfSkipKVs - || getComparator().compareRows(cur, firstKeyOfPreviousRow) > 0) { - return seekToPreviousRow(firstKeyOfPreviousRow); - } - + boolean recursiveSeek = false; + Cell key = originalKey; + do { + KeyValue seekKey = KeyValueUtil.createFirstOnRow(key.getRowArray(), key.getRowOffset(), + key.getRowLength()); + if (seekCount != null) seekCount.incrementAndGet(); + if (!hfs.seekBefore(seekKey.getBuffer(), seekKey.getKeyOffset(), + seekKey.getKeyLength())) { + close(); + return false; + } + KeyValue firstKeyOfPreviousRow = KeyValueUtil.createFirstOnRow(hfs.getKeyValue() + .getRowArray(), hfs.getKeyValue().getRowOffset(), hfs.getKeyValue().getRowLength()); + + if (seekCount != null) seekCount.incrementAndGet(); + if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { + close(); + return false; + } + + setCurrentCell(hfs.getKeyValue()); + this.stopSkippingKVsIfNextRow = true; + boolean resultOfSkipKVs; + try { + resultOfSkipKVs = skipKVsNewerThanReadpoint(); + } finally { + this.stopSkippingKVsIfNextRow = false; + } + if (!resultOfSkipKVs + || getComparator().compareRows(cur, firstKeyOfPreviousRow) > 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; @@ -478,7 +486,7 @@ public class StoreFileScanner implements KeyValueScanner { throw e; } 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 5b9f63e..8da8af3 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 @@ -5750,6 +5750,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(true, true); + + 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(true, true); + + 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)