HBASE-3758 Delete triggers pre/postScannerOpen upcalls of RegionObserver

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1092842 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -73,6 +73,8 @@ Release 0.91.0 - Unreleased
                and some keyvalue is outdated (Zhou Shuaifeng via Stack)
    HBASE-3624  Only one coprocessor of each priority can be loaded for a table
    HBASE-3598  Broken formatting in LRU stats output (Erik Onnen)
+   HBASE-3758  Delete triggers pre/postScannerOpen upcalls of RegionObserver
+               (Mingjie Lai via garyh)
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  

src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

@@ -1199,19 +1199,25 @@ public class HRegion implements HeapSize { // , Writable{
    return getScanner(scan, null);
   }
 
+  void prepareScanner(Scan scan) throws IOException {
+    if(!scan.hasFamilies()) {
+      // Adding all families to scanner
+      for(byte[] family: regionInfo.getTableDesc().getFamiliesKeys()){
+        scan.addFamily(family);
+      }
+    }
+  }
+
   protected InternalScanner getScanner(Scan scan, List<KeyValueScanner> additionalScanners) throws IOException {
     startRegionOperation();
     this.readRequestsCount.increment();
     try {
       // Verify families are all valid
+      prepareScanner(scan);
       if(scan.hasFamilies()) {
         for(byte [] family : scan.getFamilyMap().keySet()) {
           checkFamily(family);
         }
-      } else { // Adding all families to scanner
-        for(byte[] family: regionInfo.getTableDesc().getFamiliesKeys()){
-          scan.addFamily(family);
-        }
       }
       return instantiateInternalScanner(scan, additionalScanners);
 
@@ -1222,17 +1228,7 @@ public class HRegion implements HeapSize { // , Writable{
 
   protected InternalScanner instantiateInternalScanner(Scan scan,
       List<KeyValueScanner> additionalScanners) throws IOException {
-    InternalScanner s = null;
+    return new RegionScanner(scan, additionalScanners);
-    if (coprocessorHost != null) {
-      s = coprocessorHost.preScannerOpen(scan);
-    }
-    if (s == null) {
-      s = new RegionScanner(scan, additionalScanners);
-    }
-    if (coprocessorHost != null) {
-      s = coprocessorHost.postScannerOpen(scan, s);
-    }
-    return s;
   }
 
   /*

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -1882,7 +1882,18 @@ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler,
     requestCount.incrementAndGet();
     try {
       HRegion r = getRegion(regionName);
-      return addScanner(r.getScanner(scan));
+      r.prepareScanner(scan);
+      InternalScanner s = null;
+      if (r.getCoprocessorHost() != null) {
+        s = r.getCoprocessorHost().preScannerOpen(scan);
+      }
+      if (s == null) {
+        s = r.getScanner(scan);
+      }
+      if (r.getCoprocessorHost() != null) {
+        s = r.getCoprocessorHost().postScannerOpen(scan, s);
+      }
+      return addScanner(s);
     } catch (Throwable t) {
       throw convertThrowableToIOE(cleanup(t, "Failed openScanner"));
     }

src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java

@@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.Increment;
 import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.InternalScanner;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -73,6 +74,8 @@ public class SimpleRegionObserver extends BaseRegionObserverCoprocessor {
   boolean hadPostScannerNext = false;
   boolean hadPreScannerClose = false;
   boolean hadPostScannerClose = false;
+  boolean hadPreScannerOpen = false;
+  boolean hadPostScannerOpen = false;
 
   @Override
   public void preOpen(ObserverContext<RegionCoprocessorEnvironment> c) {
@@ -140,6 +143,21 @@ public class SimpleRegionObserver extends BaseRegionObserverCoprocessor {
     hadPostCompact = true;
   }
 
+  @Override
+  public InternalScanner preScannerOpen(final ObserverContext<RegionCoprocessorEnvironment> c,
+      final Scan scan,
+      final InternalScanner s) throws IOException {
+    hadPreScannerOpen = true;
+    return null;
+  }
+
+  @Override
+  public InternalScanner postScannerOpen(final ObserverContext<RegionCoprocessorEnvironment> c,
+      final Scan scan, final InternalScanner s)
+      throws IOException {
+    hadPostScannerOpen = true;
+    return s;
+  }
 
   @Override
   public boolean preScannerNext(final ObserverContext<RegionCoprocessorEnvironment> c,
@@ -372,4 +390,10 @@ public class SimpleRegionObserver extends BaseRegionObserverCoprocessor {
   public boolean wasScannerCloseCalled() {
     return hadPreScannerClose && hadPostScannerClose;
   }
+  public boolean wasScannerOpenCalled() {
+    return hadPreScannerOpen && hadPostScannerOpen;
+  }
+  public boolean hadDeleted() {
+    return hadPreDeleted && hadPostDeleted;
+  }
 }

src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java

@@ -206,6 +206,50 @@ public class TestRegionObserverInterface {
     util.deleteTable(tableName);
   }
 
+  @Test
+  // HBase-3758
+  public void testHBase3758() throws IOException {
+    byte[] tableName = Bytes.toBytes("testHBase3758");
+    util.createTable(tableName, new byte[][] {A, B, C});
+
+    verifyMethodResult(SimpleRegionObserver.class,
+        new String[] {"hadDeleted", "wasScannerOpenCalled"},
+        tableName,
+        new Boolean[] {false, false}
+    );
+
+    HTable table = new HTable(util.getConfiguration(), tableName);
+    Put put = new Put(ROW);
+    put.add(A, A, A);
+    table.put(put);
+
+    Delete delete = new Delete(ROW);
+    table.delete(delete);
+
+    verifyMethodResult(SimpleRegionObserver.class,
+        new String[] {"hadDeleted", "wasScannerOpenCalled"},
+        tableName,
+        new Boolean[] {true, false}
+    );
+
+    Scan s = new Scan();
+    ResultScanner scanner = table.getScanner(s);
+    try {
+      for (Result rr = scanner.next(); rr != null; rr = scanner.next()) {
+      }
+    } finally {
+      scanner.close();
+    }
+
+    // now scanner hooks should be invoked.
+    verifyMethodResult(SimpleRegionObserver.class,
+        new String[] {"wasScannerOpenCalled"},
+        tableName,
+        new Boolean[] {true}
+    );
+    util.deleteTable(tableName);
+  }
+
   // check each region whether the coprocessor upcalls are called or not.
   private void verifyMethodResult(Class c, String methodName[], byte[] tableName,
                                   Object value[]) throws IOException {