You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2018/07/23 20:10:55 UTC

[1/4] hbase git commit: HBASE-17885 Backport HBASE-15871 to branch-1

Repository: hbase
Updated Branches:
  refs/heads/branch-1 a7fe71da8 -> 2eaa24a13
  refs/heads/branch-1.2 79d716476 -> 756bbedb7
  refs/heads/branch-1.3 0981cd0a6 -> aafd1ac19
  refs/heads/branch-1.4 cd04cd241 -> f7e8dcdc1


HBASE-17885 Backport HBASE-15871 to branch-1

HBASE-15871 Memstore flush doesn't finish because of backwardseek() in memstore scanner (ramkrishna.s.vasudevan)

Signed-off-by: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/2eaa24a1
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/2eaa24a1
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/2eaa24a1

Branch: refs/heads/branch-1
Commit: 2eaa24a132337ca931727cbaadac25f77154a904
Parents: a7fe71d
Author: Toshihiro Suzuki <br...@gmail.com>
Authored: Fri Jul 20 10:44:26 2018 +0900
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Jul 23 12:27:52 2018 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/DefaultMemStore.java     | 10 +++-
 .../hadoop/hbase/regionserver/HStore.java       |  4 +-
 .../mapreduce/TestLoadIncrementalHFiles.java    |  2 +-
 .../hbase/regionserver/TestDefaultMemStore.java |  5 +-
 .../hadoop/hbase/regionserver/TestHRegion.java  | 61 ++++++++++++++++++++
 5 files changed, 75 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/2eaa24a1/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
index b1e9f32..edc6511 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
@@ -647,8 +647,14 @@ public class DefaultMemStore implements MemStore {
    */
   @Override
   public List<KeyValueScanner> getScanners(long readPt) {
-    return Collections.<KeyValueScanner> singletonList(
-        new MemStoreScanner(activeSection, snapshotSection, readPt, comparator));
+    MemStoreScanner scanner =
+      new MemStoreScanner(activeSection, snapshotSection, readPt, comparator);
+    scanner.seek(CellUtil.createCell(HConstants.EMPTY_START_ROW));
+    if (scanner.peek() == null) {
+      scanner.close();
+      return null;
+    }
+    return Collections.<KeyValueScanner> singletonList(scanner);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/2eaa24a1/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index ccc7be6..9ddd037 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -1220,7 +1220,9 @@ public class HStore implements Store {
       new ArrayList<KeyValueScanner>(sfScanners.size()+1);
     scanners.addAll(sfScanners);
     // Then the memstore scanners
-    scanners.addAll(memStoreScanners);
+    if (memStoreScanners != null) {
+      scanners.addAll(memStoreScanners);
+    }
     return scanners;
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/2eaa24a1/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
index 13a2274..bc87c8d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
@@ -177,7 +177,7 @@ public class TestLoadIncrementalHFiles {
    * Test case that creates some regions and loads HFiles that cross the boundaries
    * and have different region boundaries than the table pre-split.
    */
-  @Test(timeout = 60000)
+  @Test(timeout = 80000)
   public void testRegionCrossingHFileSplit() throws Exception {
     testRegionCrossingHFileSplit(BloomType.NONE);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/2eaa24a1/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
index 6e53a64..d0ee86b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
@@ -276,12 +276,11 @@ public class TestDefaultMemStore extends TestCase {
     kv1.setSequenceId(w.getWriteNumber());
     memstore.add(kv1);
 
-    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
-    assertScannerResults(s, new KeyValue[]{});
+    assertTrue(this.memstore.getScanners(mvcc.getReadPoint()) == null);
 
     mvcc.completeAndWait(w);
 
-    s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
+    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
     assertScannerResults(s, new KeyValue[]{kv1});
 
     w = mvcc.begin();

http://git-wip-us.apache.org/repos/asf/hbase/blob/2eaa24a1/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
----------------------------------------------------------------------
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 9c3b7af8..f7d57b5 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
@@ -6809,4 +6809,65 @@ public class TestHRegion {
       this.region = null;
     }
   }
+
+  @Test
+  public void testReverseScanShouldNotScanMemstoreIfReadPtLesser() throws Exception {
+    byte[] cf1 = Bytes.toBytes("CF1");
+    byte[][] families = { cf1 };
+    byte[] col = Bytes.toBytes("C");
+    String method = this.getName();
+    HBaseConfiguration conf = new HBaseConfiguration();
+    this.region = initHRegion(tableName, method, conf, families);
+    try {
+      // setup with one storefile and one memstore, to create scanner and get an earlier readPt
+      Put put = new Put(Bytes.toBytes("19996"));
+      put.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put);
+      Put put2 = new Put(Bytes.toBytes("19995"));
+      put2.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put2);
+
+      // create a reverse scan
+      Scan scan = new Scan(Bytes.toBytes("19996"));
+      scan.setReversed(true);
+      RegionScanner scanner = region.getScanner(scan);
+
+      // flush the cache. This will reset the store scanner
+      region.flushcache(true, true);
+
+      // create one memstore contains many rows will be skipped
+      // to check MemStoreScanner.seekToPreviousRow
+      for (int i = 10000; i < 20000; i++) {
+        Put p = new Put(Bytes.toBytes("" + i));
+        p.addColumn(cf1, col, Bytes.toBytes("" + i));
+        region.put(p);
+      }
+
+      List<Cell> currRow = new ArrayList<>();
+      boolean hasNext;
+      boolean assertDone = false;
+      do {
+        hasNext = scanner.next(currRow);
+        // With HBASE-15871, after the scanner is reset the memstore scanner should not be
+        // added here
+        if (!assertDone) {
+          StoreScanner current =
+            (StoreScanner) (((RegionScannerImpl) scanner).storeHeap).getCurrentForTesting();
+          List<KeyValueScanner> scanners = current.getAllScannersForTesting();
+          assertEquals("There should be only one scanner the store file scanner", 1,
+            scanners.size());
+          assertDone = true;
+        }
+      } while (hasNext);
+
+      assertEquals(2, currRow.size());
+      assertEquals("19996", Bytes.toString(currRow.get(0).getRowArray(),
+        currRow.get(0).getRowOffset(), currRow.get(0).getRowLength()));
+      assertEquals("19995", Bytes.toString(currRow.get(1).getRowArray(),
+        currRow.get(1).getRowOffset(), currRow.get(1).getRowLength()));
+    } finally {
+      HBaseTestingUtility.closeRegionAndWAL(this.region);
+      this.region = null;
+    }
+  }
 }


[2/4] hbase git commit: HBASE-17885 Backport HBASE-15871 to branch-1

Posted by ap...@apache.org.
HBASE-17885 Backport HBASE-15871 to branch-1

HBASE-15871 Memstore flush doesn't finish because of backwardseek() in memstore scanner (ramkrishna.s.vasudevan)

Signed-off-by: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/f7e8dcdc
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/f7e8dcdc
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/f7e8dcdc

Branch: refs/heads/branch-1.4
Commit: f7e8dcdc1926516cff6ead198119c16aad3d433b
Parents: cd04cd2
Author: Toshihiro Suzuki <br...@gmail.com>
Authored: Fri Jul 20 10:44:26 2018 +0900
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Jul 23 12:29:07 2018 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/DefaultMemStore.java     | 10 +++-
 .../hadoop/hbase/regionserver/HStore.java       |  4 +-
 .../mapreduce/TestLoadIncrementalHFiles.java    |  2 +-
 .../hbase/regionserver/TestDefaultMemStore.java |  5 +-
 .../hadoop/hbase/regionserver/TestHRegion.java  | 61 ++++++++++++++++++++
 5 files changed, 75 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/f7e8dcdc/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
index b1e9f32..edc6511 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
@@ -647,8 +647,14 @@ public class DefaultMemStore implements MemStore {
    */
   @Override
   public List<KeyValueScanner> getScanners(long readPt) {
-    return Collections.<KeyValueScanner> singletonList(
-        new MemStoreScanner(activeSection, snapshotSection, readPt, comparator));
+    MemStoreScanner scanner =
+      new MemStoreScanner(activeSection, snapshotSection, readPt, comparator);
+    scanner.seek(CellUtil.createCell(HConstants.EMPTY_START_ROW));
+    if (scanner.peek() == null) {
+      scanner.close();
+      return null;
+    }
+    return Collections.<KeyValueScanner> singletonList(scanner);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/f7e8dcdc/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index 804da38..08a0240 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -1209,7 +1209,9 @@ public class HStore implements Store {
       new ArrayList<KeyValueScanner>(sfScanners.size()+1);
     scanners.addAll(sfScanners);
     // Then the memstore scanners
-    scanners.addAll(memStoreScanners);
+    if (memStoreScanners != null) {
+      scanners.addAll(memStoreScanners);
+    }
     return scanners;
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/f7e8dcdc/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
index 13a2274..bc87c8d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
@@ -177,7 +177,7 @@ public class TestLoadIncrementalHFiles {
    * Test case that creates some regions and loads HFiles that cross the boundaries
    * and have different region boundaries than the table pre-split.
    */
-  @Test(timeout = 60000)
+  @Test(timeout = 80000)
   public void testRegionCrossingHFileSplit() throws Exception {
     testRegionCrossingHFileSplit(BloomType.NONE);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/f7e8dcdc/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
index 6e53a64..d0ee86b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
@@ -276,12 +276,11 @@ public class TestDefaultMemStore extends TestCase {
     kv1.setSequenceId(w.getWriteNumber());
     memstore.add(kv1);
 
-    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
-    assertScannerResults(s, new KeyValue[]{});
+    assertTrue(this.memstore.getScanners(mvcc.getReadPoint()) == null);
 
     mvcc.completeAndWait(w);
 
-    s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
+    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
     assertScannerResults(s, new KeyValue[]{kv1});
 
     w = mvcc.begin();

http://git-wip-us.apache.org/repos/asf/hbase/blob/f7e8dcdc/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
----------------------------------------------------------------------
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 09d0b4f..f2d9e80 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
@@ -6809,4 +6809,65 @@ public class TestHRegion {
       this.region = null;
     }
   }
+
+  @Test
+  public void testReverseScanShouldNotScanMemstoreIfReadPtLesser() throws Exception {
+    byte[] cf1 = Bytes.toBytes("CF1");
+    byte[][] families = { cf1 };
+    byte[] col = Bytes.toBytes("C");
+    String method = this.getName();
+    HBaseConfiguration conf = new HBaseConfiguration();
+    this.region = initHRegion(tableName, method, conf, families);
+    try {
+      // setup with one storefile and one memstore, to create scanner and get an earlier readPt
+      Put put = new Put(Bytes.toBytes("19996"));
+      put.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put);
+      Put put2 = new Put(Bytes.toBytes("19995"));
+      put2.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put2);
+
+      // create a reverse scan
+      Scan scan = new Scan(Bytes.toBytes("19996"));
+      scan.setReversed(true);
+      RegionScanner scanner = region.getScanner(scan);
+
+      // flush the cache. This will reset the store scanner
+      region.flushcache(true, true);
+
+      // create one memstore contains many rows will be skipped
+      // to check MemStoreScanner.seekToPreviousRow
+      for (int i = 10000; i < 20000; i++) {
+        Put p = new Put(Bytes.toBytes("" + i));
+        p.addColumn(cf1, col, Bytes.toBytes("" + i));
+        region.put(p);
+      }
+
+      List<Cell> currRow = new ArrayList<>();
+      boolean hasNext;
+      boolean assertDone = false;
+      do {
+        hasNext = scanner.next(currRow);
+        // With HBASE-15871, after the scanner is reset the memstore scanner should not be
+        // added here
+        if (!assertDone) {
+          StoreScanner current =
+            (StoreScanner) (((RegionScannerImpl) scanner).storeHeap).getCurrentForTesting();
+          List<KeyValueScanner> scanners = current.getAllScannersForTesting();
+          assertEquals("There should be only one scanner the store file scanner", 1,
+            scanners.size());
+          assertDone = true;
+        }
+      } while (hasNext);
+
+      assertEquals(2, currRow.size());
+      assertEquals("19996", Bytes.toString(currRow.get(0).getRowArray(),
+        currRow.get(0).getRowOffset(), currRow.get(0).getRowLength()));
+      assertEquals("19995", Bytes.toString(currRow.get(1).getRowArray(),
+        currRow.get(1).getRowOffset(), currRow.get(1).getRowLength()));
+    } finally {
+      HBaseTestingUtility.closeRegionAndWAL(this.region);
+      this.region = null;
+    }
+  }
 }


[4/4] hbase git commit: HBASE-17885 Backport HBASE-15871 to branch-1

Posted by ap...@apache.org.
HBASE-17885 Backport HBASE-15871 to branch-1

HBASE-15871 Memstore flush doesn't finish because of backwardseek() in memstore scanner (ramkrishna.s.vasudevan)

Signed-off-by: Andrew Purtell <ap...@apache.org>

Conflicts:
	hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
	hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/756bbedb
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/756bbedb
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/756bbedb

Branch: refs/heads/branch-1.2
Commit: 756bbedb78a8aa3dc0d86c3c1143f021a121580f
Parents: 79d7164
Author: Toshihiro Suzuki <br...@gmail.com>
Authored: Fri Jul 20 10:44:26 2018 +0900
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Jul 23 12:41:57 2018 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/DefaultMemStore.java     |  8 ++-
 .../hadoop/hbase/regionserver/HStore.java       |  4 +-
 .../mapreduce/TestLoadIncrementalHFiles.java    |  2 +-
 .../hbase/regionserver/TestDefaultMemStore.java |  5 +-
 .../hadoop/hbase/regionserver/TestHRegion.java  | 61 ++++++++++++++++++++
 5 files changed, 74 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/756bbedb/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
index 6d642dc..3d095cb 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
@@ -680,7 +680,13 @@ public class DefaultMemStore implements MemStore {
    */
   @Override
   public List<KeyValueScanner> getScanners(long readPt) {
-    return Collections.<KeyValueScanner> singletonList(new MemStoreScanner(readPt));
+    MemStoreScanner scanner = new MemStoreScanner(readPt);
+    scanner.seek(CellUtil.createCell(HConstants.EMPTY_START_ROW));
+    if (scanner.peek() == null) {
+      scanner.close();
+      return null;
+    }
+    return Collections.<KeyValueScanner> singletonList(scanner);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/756bbedb/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index 09cb50d..5de73b3 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -1126,7 +1126,9 @@ public class HStore implements Store {
       new ArrayList<KeyValueScanner>(sfScanners.size()+1);
     scanners.addAll(sfScanners);
     // Then the memstore scanners
-    scanners.addAll(memStoreScanners);
+    if (memStoreScanners != null) {
+      scanners.addAll(memStoreScanners);
+    }
     return scanners;
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/756bbedb/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
index 4f1fc0f..e25b898 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
@@ -179,7 +179,7 @@ public class TestLoadIncrementalHFiles {
    * Test case that creates some regions and loads HFiles that cross the boundaries
    * and have different region boundaries than the table pre-split.
    */
-  @Test(timeout = 60000)
+  @Test(timeout = 80000)
   public void testRegionCrossingHFileSplit() throws Exception {
     testRegionCrossingHFileSplit(BloomType.NONE);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/756bbedb/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
index c4d7f91..0fa746e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
@@ -275,12 +275,11 @@ public class TestDefaultMemStore extends TestCase {
     kv1.setSequenceId(w.getWriteNumber());
     memstore.add(kv1);
 
-    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
-    assertScannerResults(s, new KeyValue[]{});
+    assertTrue(this.memstore.getScanners(mvcc.getReadPoint()) == null);
 
     mvcc.completeAndWait(w);
 
-    s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
+    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
     assertScannerResults(s, new KeyValue[]{kv1});
 
     w = mvcc.begin();

http://git-wip-us.apache.org/repos/asf/hbase/blob/756bbedb/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
----------------------------------------------------------------------
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 62d9382..d6c2f47 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
@@ -6699,4 +6699,65 @@ public class TestHRegion {
       this.region = null;
     }
   }
+
+  @Test
+  public void testReverseScanShouldNotScanMemstoreIfReadPtLesser() throws Exception {
+    byte[] cf1 = Bytes.toBytes("CF1");
+    byte[][] families = { cf1 };
+    byte[] col = Bytes.toBytes("C");
+    String method = this.getName();
+    HBaseConfiguration conf = new HBaseConfiguration();
+    this.region = initHRegion(tableName, method, conf, families);
+    try {
+      // setup with one storefile and one memstore, to create scanner and get an earlier readPt
+      Put put = new Put(Bytes.toBytes("19996"));
+      put.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put);
+      Put put2 = new Put(Bytes.toBytes("19995"));
+      put2.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put2);
+
+      // create a reverse scan
+      Scan scan = new Scan(Bytes.toBytes("19996"));
+      scan.setReversed(true);
+      RegionScanner scanner = region.getScanner(scan);
+
+      // flush the cache. This will reset the store scanner
+      region.flushcache(true, true);
+
+      // create one memstore contains many rows will be skipped
+      // to check MemStoreScanner.seekToPreviousRow
+      for (int i = 10000; i < 20000; i++) {
+        Put p = new Put(Bytes.toBytes("" + i));
+        p.addColumn(cf1, col, Bytes.toBytes("" + i));
+        region.put(p);
+      }
+
+      List<Cell> currRow = new ArrayList<>();
+      boolean hasNext;
+      boolean assertDone = false;
+      do {
+        hasNext = scanner.next(currRow);
+        // With HBASE-15871, after the scanner is reset the memstore scanner should not be
+        // added here
+        if (!assertDone) {
+          StoreScanner current =
+            (StoreScanner) (((RegionScannerImpl) scanner).storeHeap).getCurrentForTesting();
+          List<KeyValueScanner> scanners = current.getAllScannersForTesting();
+          assertEquals("There should be only one scanner the store file scanner", 1,
+            scanners.size());
+          assertDone = true;
+        }
+      } while (hasNext);
+
+      assertEquals(2, currRow.size());
+      assertEquals("19996", Bytes.toString(currRow.get(0).getRowArray(),
+        currRow.get(0).getRowOffset(), currRow.get(0).getRowLength()));
+      assertEquals("19995", Bytes.toString(currRow.get(1).getRowArray(),
+        currRow.get(1).getRowOffset(), currRow.get(1).getRowLength()));
+    } finally {
+      HBaseTestingUtility.closeRegionAndWAL(this.region);
+      this.region = null;
+    }
+  }
 }


[3/4] hbase git commit: HBASE-17885 Backport HBASE-15871 to branch-1

Posted by ap...@apache.org.
HBASE-17885 Backport HBASE-15871 to branch-1

HBASE-15871 Memstore flush doesn't finish because of backwardseek() in memstore scanner (ramkrishna.s.vasudevan)

Signed-off-by: Andrew Purtell <ap...@apache.org>

Conflicts:
	hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/aafd1ac1
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/aafd1ac1
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/aafd1ac1

Branch: refs/heads/branch-1.3
Commit: aafd1ac19034524152258c7c5f60077a1975f890
Parents: 0981cd0
Author: Toshihiro Suzuki <br...@gmail.com>
Authored: Fri Jul 20 10:44:26 2018 +0900
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Jul 23 12:29:41 2018 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/DefaultMemStore.java     | 10 +++-
 .../hadoop/hbase/regionserver/HStore.java       |  4 +-
 .../mapreduce/TestLoadIncrementalHFiles.java    |  2 +-
 .../hbase/regionserver/TestDefaultMemStore.java |  5 +-
 .../hadoop/hbase/regionserver/TestHRegion.java  | 61 ++++++++++++++++++++
 5 files changed, 75 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/aafd1ac1/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
index 884ef29..31922ae 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
@@ -632,8 +632,14 @@ public class DefaultMemStore implements MemStore {
    */
   @Override
   public List<KeyValueScanner> getScanners(long readPt) {
-    return Collections.<KeyValueScanner> singletonList(
-        new MemStoreScanner(activeSection, snapshotSection, readPt, comparator));
+    MemStoreScanner scanner =
+      new MemStoreScanner(activeSection, snapshotSection, readPt, comparator);
+    scanner.seek(CellUtil.createCell(HConstants.EMPTY_START_ROW));
+    if (scanner.peek() == null) {
+      scanner.close();
+      return null;
+    }
+    return Collections.<KeyValueScanner> singletonList(scanner);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/aafd1ac1/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index bc6ec6c..a30e827 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -1180,7 +1180,9 @@ public class HStore implements Store {
       new ArrayList<KeyValueScanner>(sfScanners.size()+1);
     scanners.addAll(sfScanners);
     // Then the memstore scanners
-    scanners.addAll(memStoreScanners);
+    if (memStoreScanners != null) {
+      scanners.addAll(memStoreScanners);
+    }
     return scanners;
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/aafd1ac1/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
index 4f1fc0f..e25b898 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
@@ -179,7 +179,7 @@ public class TestLoadIncrementalHFiles {
    * Test case that creates some regions and loads HFiles that cross the boundaries
    * and have different region boundaries than the table pre-split.
    */
-  @Test(timeout = 60000)
+  @Test(timeout = 80000)
   public void testRegionCrossingHFileSplit() throws Exception {
     testRegionCrossingHFileSplit(BloomType.NONE);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/aafd1ac1/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
index f11f7cf..ba192b6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
@@ -275,12 +275,11 @@ public class TestDefaultMemStore extends TestCase {
     kv1.setSequenceId(w.getWriteNumber());
     memstore.add(kv1);
 
-    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
-    assertScannerResults(s, new KeyValue[]{});
+    assertTrue(this.memstore.getScanners(mvcc.getReadPoint()) == null);
 
     mvcc.completeAndWait(w);
 
-    s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
+    KeyValueScanner s = this.memstore.getScanners(mvcc.getReadPoint()).get(0);
     assertScannerResults(s, new KeyValue[]{kv1});
 
     w = mvcc.begin();

http://git-wip-us.apache.org/repos/asf/hbase/blob/aafd1ac1/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
----------------------------------------------------------------------
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 3d1ec1c..11fc26e 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
@@ -6698,4 +6698,65 @@ public class TestHRegion {
       this.region = null;
     }
   }
+
+  @Test
+  public void testReverseScanShouldNotScanMemstoreIfReadPtLesser() throws Exception {
+    byte[] cf1 = Bytes.toBytes("CF1");
+    byte[][] families = { cf1 };
+    byte[] col = Bytes.toBytes("C");
+    String method = this.getName();
+    HBaseConfiguration conf = new HBaseConfiguration();
+    this.region = initHRegion(tableName, method, conf, families);
+    try {
+      // setup with one storefile and one memstore, to create scanner and get an earlier readPt
+      Put put = new Put(Bytes.toBytes("19996"));
+      put.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put);
+      Put put2 = new Put(Bytes.toBytes("19995"));
+      put2.addColumn(cf1, col, Bytes.toBytes("val"));
+      region.put(put2);
+
+      // create a reverse scan
+      Scan scan = new Scan(Bytes.toBytes("19996"));
+      scan.setReversed(true);
+      RegionScanner scanner = region.getScanner(scan);
+
+      // flush the cache. This will reset the store scanner
+      region.flushcache(true, true);
+
+      // create one memstore contains many rows will be skipped
+      // to check MemStoreScanner.seekToPreviousRow
+      for (int i = 10000; i < 20000; i++) {
+        Put p = new Put(Bytes.toBytes("" + i));
+        p.addColumn(cf1, col, Bytes.toBytes("" + i));
+        region.put(p);
+      }
+
+      List<Cell> currRow = new ArrayList<>();
+      boolean hasNext;
+      boolean assertDone = false;
+      do {
+        hasNext = scanner.next(currRow);
+        // With HBASE-15871, after the scanner is reset the memstore scanner should not be
+        // added here
+        if (!assertDone) {
+          StoreScanner current =
+            (StoreScanner) (((RegionScannerImpl) scanner).storeHeap).getCurrentForTesting();
+          List<KeyValueScanner> scanners = current.getAllScannersForTesting();
+          assertEquals("There should be only one scanner the store file scanner", 1,
+            scanners.size());
+          assertDone = true;
+        }
+      } while (hasNext);
+
+      assertEquals(2, currRow.size());
+      assertEquals("19996", Bytes.toString(currRow.get(0).getRowArray(),
+        currRow.get(0).getRowOffset(), currRow.get(0).getRowLength()));
+      assertEquals("19995", Bytes.toString(currRow.get(1).getRowArray(),
+        currRow.get(1).getRowOffset(), currRow.get(1).getRowLength()));
+    } finally {
+      HBaseTestingUtility.closeRegionAndWAL(this.region);
+      this.region = null;
+    }
+  }
 }