Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1588535 - in /hbase/branches/0.89-fb/src:
 main/java/org/apache/hadoop/hbase/client/
 main/java/org/apache/hadoop/hbase/regionserver/
 main/java/org/apache/hadoop/hbase/util/
 test/java/org/apache/hadoop/hbase/client/
Date: Fri, 18 Apr 2014 18:18:22 -0000
To: commits@hbase.apache.org
From: liyin@apache.org
Message-Id: <20140418181822.9691F2388860@eris.apache.org>

Author: liyin
Date: Fri Apr 18 18:18:21 2014
New Revision: 1588535

URL: http://svn.apache.org/r1588535
Log:
[HBASE-9704] Fix the HTableClientScanner when the scan is done and the region is done.

Author: manukranthk

Summary: Result[] being null marks end of scan and being [] marks end of region. But in case when both happen, we progress to the next region, which is pointless.

Test Plan: Unit test

Reviewers: rshroff, gauravm

Reviewed By: gauravm

CC: hbase-eng@, daviddeng, gauravm

Differential Revision: https://phabricator.fb.com/D1281576

Task ID: 4158431

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/client/HTableClientScanner.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/client/HTableClientScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/client/HTableClientScanner.java?rev=1588535&r1=1588534&r2=1588535&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/client/HTableClientScanner.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/client/HTableClientScanner.java Fri Apr 18 18:18:21 2014
@@ -26,6 +26,7 @@ import java.util.concurrent.ArrayBlockin
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.commons.logging.Log;
@@ -245,6 +246,16 @@ public class HTableClientScanner impleme
     return closed;
   }
 
+  /**
+   * Returns the number of regions that we've scanned during the current scan.
+   * Only used during testing, but it can be used otherwise as well.
+   * Can be incorrect in case when the connection to the region fails.
+   * @return
+   */
+  int getNumRegionsScanned() {
+    return this.fetcher.numRegionsScanned.get();
+  }
+
   private static class Fetcher implements Runnable {
     // The startKey for opening a scanner.
     private byte[] startKey;
@@ -265,6 +276,7 @@ public class HTableClientScanner impleme
     private final AtomicReference<Result[]> justFetched;
     private final AtomicReference<Throwable> exception;
     private final AtomicBoolean closing;
+    private final AtomicInteger numRegionsScanned = new AtomicInteger(0);
 
     public Fetcher(HTable table, Scan scan, int caching,
         ArrayBlockingQueue<Result[]> queue,
@@ -337,6 +349,7 @@ public class HTableClientScanner impleme
 
         lastRes = null;
         lastSuccNextTs = System.currentTimeMillis();
+        this.numRegionsScanned.addAndGet(1);
       }
 
       boolean keepCallable = false;

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java?rev=1588535&r1=1588534&r2=1588535&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java Fri Apr 18 18:18:21 2014
@@ -267,7 +267,7 @@ public class RegionScanner implements In
     preCondition();
     boolean prefetchingEnabled = getOriginalScan().getServerPrefetching();
     int limit = this.getOriginalScan().getBatch();
-    ScanResult scanResult;
+    ScanResult scanResult = null;
     // if we have a prefetched result, then use it
     if (prefetchingEnabled && prefetchScanFuture != null) {
       try {
@@ -277,6 +277,10 @@ public class RegionScanner implements In
         throw new IOException(e);
       } catch (ExecutionException e) {
         throw new IOException(e);
+      } finally {
+        if (scanResult == null) {
+          scanResult = new ScanResult(new IOException("Unknown Exception"));
+        }
       }
       if (scanResult.isException) {
         throw scanResult.ioException;
@@ -296,11 +300,23 @@ public class RegionScanner implements In
       ScanPrefetcher callable = new ScanPrefetcher(nbRows, limit, metric);
       prefetchScanFuture = scanPrefetchThreadPool.submit(callable);
     }
-    rowReadCnt.addAndGet(scanResult.outResults.length);
-    Result[] ret;
-    if (scanResult.outResults == null ||
-        (isFilterDone() && scanResult.outResults.length == 0)) {
-      ret = Result.SENTINEL_RESULT_ARRAY;
+    if (scanResult.outResults != null) {
+      rowReadCnt.addAndGet(scanResult.outResults.length);
+    }
+    Result[] ret = Result.SENTINEL_RESULT_ARRAY;
+    if (scanResult.outResults == null) {
+      return ret;
+    } else if (scanResult.outResults.length == 0) {
+      // We need to return Result.SENTINEL_RESULT_ARRAY to terminate the
+      // scan if isFilterDone()
+      if (!isFilterDone()) {
+        // In case then isFilterDone is false, do a sanity check
+        // to see if it makes sense to continue.
+        if (this.stopRow == null ||
+            Bytes.compareTo(this.regionInfo.getEndKey(), this.stopRow) > 0) {
+          ret = scanResult.outResults;
+        }
+      }
     } else {
       ret = scanResult.outResults;
     }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/Bytes.java?rev=1588535&r1=1588534&r2=1588535&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/Bytes.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/Bytes.java Fri Apr 18 18:18:21 2014
@@ -1836,6 +1836,9 @@ public class Bytes {
    * Return whether b equals nextOf(a)
    */
   public static boolean isNext(byte[] a, byte[] b) {
+    if (a == null || b == null) {
+      return false;
+    }
     return isNext(a, 0, a.length, b, 0, b.length);
   }
 

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java?rev=1588535&r1=1588534&r2=1588535&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java Fri Apr 18 18:18:21 2014
@@ -82,6 +82,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.weakref.jmx.com.google.common.collect.Lists;
 
 /**
  * Run tests that use the HBase clients; {@link HTable} and {@link HTablePool}.
@@ -4515,5 +4516,36 @@ public class TestFromClientSide {
     assertTrue(Bytes.equals(result.getKvs().iterator().next().getQualifier(),
         QUALIFIERS[0]));
   }
+
+  /**
+   * Tests the case where the scan is done and the region is done, but the scan
+   * jumps to the next region.
+   * @throws InterruptedException
+   * @throws IOException
+   */
+  @Test
+  public void testScanDoneRegionDone() throws IOException, InterruptedException
+  {
+    final String TABLENAME = "testScanDoneRegionDone";
+    final String cf = "cf";
+    HTable t = TEST_UTIL.createRandomTable(TABLENAME,
+        Lists.asList(cf, new String[0]), 1, 100, 1, 10, 10);
+    Random rand = new Random();
+    int regionId = rand.nextInt(5);
+    Scan s = new Scan();
+    HRegionInfo info = t.getRegionsInfo().keySet()
+        .toArray(new HRegionInfo[0])[regionId];
+    s.setStartRow(info.getStartKey());
+    s.setStopRow(info.getEndKey());
+    try (ResultScanner scanner = t.getScanner(s)) {
+      if (scanner instanceof HTableClientScanner) {
+        for (Result r : scanner) {
+          // Do nothing.
+        }
+        assertEquals(1,
+            ((HTableClientScanner) scanner).getNumRegionsScanned());
+      }
+    }
+  }
 }