From 56ffb27cb1bae800f897986daedd6df5cdc75441 Mon Sep 17 00:00:00 2001 From: sunyerui Date: Sun, 27 Sep 2015 19:26:34 +0800 Subject: [PATCH] HBASE-14497 Reverse Scan threw StackOverflow caused by readPt checking --- .../hbase/regionserver/StoreFileScanner.java | 65 ++++++++++++---------- .../hadoop/hbase/regionserver/TestHRegion.java | 40 +++++++++++++ 2 files changed, 75 insertions(+), 30 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 108b889..e3f427d 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 @@ -428,37 +428,42 @@ public class StoreFileScanner implements KeyValueScanner { } @Override - public boolean seekToPreviousRow(Cell key) throws IOException { + public boolean seekToPreviousRow(Cell originalKey) throws IOException { try { try { - Cell seekKey = CellUtil.createFirstOnRow(key); - if (seekCount != null) seekCount.incrementAndGet(); - if (!hfs.seekBefore(seekKey)) { - this.cur = null; - return false; - } - Cell curCell = hfs.getCell(); - Cell firstKeyOfPreviousRow = CellUtil.createFirstOnRow(curCell); - - if (seekCount != null) seekCount.incrementAndGet(); - if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { - this.cur = null; - return false; - } - - setCurrentCell(hfs.getCell()); - 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 { + Cell seekKey = CellUtil.createFirstOnRow(key); + if (seekCount != null) seekCount.incrementAndGet(); + if (!hfs.seekBefore(seekKey)) { + this.cur = null; + return false; + } + Cell curCell = hfs.getCell(); + Cell firstKeyOfPreviousRow = CellUtil.createFirstOnRow(curCell); + + if (seekCount != null) seekCount.incrementAndGet(); + if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) { + this.cur = null; + return false; + } + + setCurrentCell(hfs.getCell()); + this.stopSkippingKVsIfNextRow = true; + boolean resultOfSkipKVs; + try { + resultOfSkipKVs = skipKVsNewerThanReadpoint(); + } finally { + this.stopSkippingKVsIfNextRow = false; + } + if (!resultOfSkipKVs + || getComparator().compareRows(cur, firstKeyOfPreviousRow) > 0) { + recursiveSeek = true; + key = firstKeyOfPreviousRow; + continue; + } + } while (recursiveSeek); return true; } finally { realSeekDone = true; @@ -467,7 +472,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 cb95d6f..5e58aa5 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 @@ -5776,6 +5776,46 @@ public class TestHRegion { } } + @Test (timeout = 60000) + public void testReverseScannerStackOverflow() throws IOException, DeserializationException { + 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()); + assertEquals("19998", Bytes.toString(currRow.get(0).getRowArray(), currRow.get(0).getRowOffset(), currRow.get(0).getRowLength())); + assertEquals("09999", Bytes.toString(currRow.get(1).getRowArray(), currRow.get(1).getRowOffset(), currRow.get(1).getRowLength())); + } + @Test (timeout=60000) public void testSplitRegionWithReverseScan() throws IOException { TableName tableName = TableName.valueOf("testSplitRegionWithReverseScan"); -- 2.3.2 (Apple Git-55)