hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From e...@apache.org
Subject [1/2] hbase git commit: HBASE-17187 DoNotRetryExceptions from coprocessors should bubble up to the application
Date Tue, 07 Feb 2017 00:13:26 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1.2 9bd76426f -> 6d1082f62


HBASE-17187 DoNotRetryExceptions from coprocessors should bubble up to the application


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

Branch: refs/heads/branch-1.2
Commit: 6d1082f626d7bdc3eb246cb0b9976c84501b16cb
Parents: a07b968
Author: Enis Soztutar <enis@apache.org>
Authored: Mon Feb 6 12:00:05 2017 -0800
Committer: Enis Soztutar <enis@apache.org>
Committed: Mon Feb 6 16:11:53 2017 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/client/ClientScanner.java      | 20 +++++-
 .../hbase/regionserver/RSRpcServices.java       | 12 +++-
 .../hadoop/hbase/client/TestFromClientSide.java | 76 +++++++++++++++++++-
 3 files changed, 103 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/6d1082f6/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
index 944f44e..053814c 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
@@ -401,6 +401,9 @@ public class ClientScanner extends AbstractClientScanner {
     // it doesn't tell us otherwise. We rely on the size or count of results
     boolean serverHasMoreResults = false;
     boolean allResultsSkipped = false;
+    // Even if we are retrying due to UnknownScannerException, ScannerResetException, etc.
we should
+    // make sure that we are not retrying indefinitely.
+    int retriesLeft = getRetries();
     do {
       allResultsSkipped = false;
       try {
@@ -425,8 +428,18 @@ public class ClientScanner extends AbstractClientScanner {
         // An exception was thrown which makes any partial results that we were collecting
         // invalid. The scanner will need to be reset to the beginning of a row.
         clearPartialResults();
-        // DNRIOEs are thrown to make us break out of retries. Some types of DNRIOEs want
us
-        // to reset the scanner and come back in again.
+
+        // Unfortunately, DNRIOE is used in two different semantics.
+        // (1) The first is to close the client scanner and bubble up the exception all the
way
+        // to the application. This is preferred when the exception is really un-recoverable
+        // (like CorruptHFileException, etc). Plain DoNotRetryIOException also falls into
this
+        // bucket usually.
+        // (2) Second semantics is to close the current region scanner only, but continue
the
+        // client scanner by overriding the exception. This is usually UnknownScannerException,
+        // OutOfOrderScannerNextException, etc where the region scanner has to be closed,
but the
+        // application-level ClientScanner has to continue without bubbling up the exception
to
+        // the client. See RSRpcServices to see how it throws DNRIOE's.
+        // See also: HBASE-16604, HBASE-17187
 
         // If exception is any but the list below throw it back to the client; else setup
         // the scanner and retry.
@@ -438,6 +451,9 @@ public class ClientScanner extends AbstractClientScanner {
             e instanceof ScannerResetException) {
           // Pass. It is easier writing the if loop test as list of what is allowed rather
than
           // as a list of what is not allowed... so if in here, it means we do not throw.
+          if (retriesLeft-- <= 0) {
+            throw e; // no more retries
+          }
         } else {
           throw e;
         }

http://git-wip-us.apache.org/repos/asf/hbase/blob/6d1082f6/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index 46a8120..1d454a1 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -2663,7 +2663,17 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
           // row that the client has last seen.
           closeScanner(region, scanner, scannerName);
 
-          // rethrow DoNotRetryIOException. This can avoid the retry in ClientScanner.
+          // If it is a DoNotRetryIOException already, throw as it is. Unfortunately, DNRIOE
is
+          // used in two different semantics.
+          // (1) The first is to close the client scanner and bubble up the exception all
the way
+          // to the application. This is preferred when the exception is really un-recoverable
+          // (like CorruptHFileException, etc). Plain DoNotRetryIOException also falls into
this
+          // bucket usually.
+          // (2) Second semantics is to close the current region scanner only, but continue
the
+          // client scanner by overriding the exception. This is usually UnknownScannerException,
+          // OutOfOrderScannerNextException, etc where the region scanner has to be closed,
but the
+          // application-level ClientScanner has to continue without bubbling up the exception
to
+          // the client. See ClientScanner code to see how it deals with these special exceptions.
           if (e instanceof DoNotRetryIOException) {
             throw e;
           }

http://git-wip-us.apache.org/repos/asf/hbase/blob/6d1082f6/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
index 0b2ba83..e8c6776 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
@@ -41,6 +41,7 @@ import java.util.UUID;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -52,6 +53,7 @@ import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.Abortable;
 import org.apache.hadoop.hbase.Cell;
 import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HColumnDescriptor;
 import org.apache.hadoop.hbase.HConstants;
@@ -70,6 +72,7 @@ import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
 import org.apache.hadoop.hbase.coprocessor.MultiRowMutationEndpoint;
 import org.apache.hadoop.hbase.coprocessor.ObserverContext;
 import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.exceptions.ScannerResetException;
 import org.apache.hadoop.hbase.filter.BinaryComparator;
 import org.apache.hadoop.hbase.filter.CompareFilter;
 import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
@@ -599,6 +602,15 @@ public class TestFromClientSide {
    */
   public static class ExceptionInReseekRegionObserver extends BaseRegionObserver {
     static AtomicLong reqCount = new AtomicLong(0);
+    static AtomicBoolean isDoNotRetry = new AtomicBoolean(false); // whether to throw DNRIOE
+    static AtomicBoolean throwOnce = new AtomicBoolean(true); // whether to only throw once
+
+    static void reset() {
+      reqCount.set(0);
+      isDoNotRetry.set(false);
+      throwOnce.set(true);
+    }
+
     class MyStoreScanner extends StoreScanner {
       public MyStoreScanner(Store store, ScanInfo scanInfo, Scan scan, NavigableSet<byte[]>
columns,
           long readPt) throws IOException {
@@ -614,8 +626,13 @@ public class TestFromClientSide {
           newScanners.add(new DelegatingKeyValueScanner(scanner) {
             @Override
             public boolean reseek(Cell key) throws IOException {
-              if (reqCount.incrementAndGet() == 1) {
-                throw new IOException("Injected exception");
+              reqCount.incrementAndGet();
+              if (!throwOnce.get()||  reqCount.get() == 1) {
+                if (isDoNotRetry.get()) {
+                  throw new DoNotRetryIOException("Injected exception");
+                } else {
+                  throw new IOException("Injected exception");
+                }
               }
               return super.reseek(key);
             }
@@ -649,6 +666,8 @@ public class TestFromClientSide {
     HTableDescriptor htd = TEST_UTIL.createTableDescriptor(name, FAMILY);
     htd.addCoprocessor(ExceptionInReseekRegionObserver.class.getName());
     TEST_UTIL.getHBaseAdmin().createTable(htd);
+    ExceptionInReseekRegionObserver.reset();
+    ExceptionInReseekRegionObserver.throwOnce.set(true); // throw exceptions only once
     try (Table t = TEST_UTIL.getConnection().getTable(name)) {
       int rowCount = TEST_UTIL.loadTable(t, FAMILY, false);
       TEST_UTIL.getHBaseAdmin().flush(name);
@@ -658,6 +677,59 @@ public class TestFromClientSide {
     assertTrue(ExceptionInReseekRegionObserver.reqCount.get() > 0);
   }
 
+  /**
+   * Tests the case where a coprocessor throws a DoNotRetryIOException in the scan. The expectation
+   * is that the exception will bubble up to the client scanner instead of being retried.
+   */
+  @Test (timeout = 180000)
+  public void testScannerThrowsExceptionWhenCoprocessorThrowsDNRIOE()
+      throws IOException, InterruptedException {
+    TEST_UTIL.getConfiguration().setBoolean("hbase.client.log.scanner.activity", true);
+    TableName name = TableName.valueOf("testClientScannerIsNotRetriedWhenCoprocessorThrowsDNRIOE");
+
+    HTableDescriptor htd = TEST_UTIL.createTableDescriptor(name, FAMILY);
+    htd.addCoprocessor(ExceptionInReseekRegionObserver.class.getName());
+    TEST_UTIL.getHBaseAdmin().createTable(htd);
+    ExceptionInReseekRegionObserver.reset();
+    ExceptionInReseekRegionObserver.isDoNotRetry.set(true);
+    try (Table t = TEST_UTIL.getConnection().getTable(name)) {
+      TEST_UTIL.loadTable(t, FAMILY, false);
+      TEST_UTIL.getHBaseAdmin().flush(name);
+      countRows(t, new Scan().addColumn(FAMILY, FAMILY));
+      fail("Should have thrown an exception");
+    } catch (RuntimeException expected) {
+      // expected
+    }
+    assertTrue(ExceptionInReseekRegionObserver.reqCount.get() > 0);
+  }
+
+  /**
+   * Tests the case where a coprocessor throws a regular IOException in the scan. The expectation
+   * is that the we will keep on retrying, but fail after the retries are exhausted instead
of
+   * retrying indefinitely.
+   */
+  @Test (timeout = 180000)
+  public void testScannerFailsAfterRetriesWhenCoprocessorThrowsIOE()
+      throws IOException, InterruptedException {
+    TEST_UTIL.getConfiguration().setBoolean("hbase.client.log.scanner.activity", true);
+    TableName name = TableName.valueOf("testScannerFailsAfterRetriesWhenCoprocessorThrowsIOE");
+    TEST_UTIL.getConfiguration().setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 3);
+    HTableDescriptor htd = TEST_UTIL.createTableDescriptor(name, FAMILY);
+    htd.addCoprocessor(ExceptionInReseekRegionObserver.class.getName());
+    TEST_UTIL.getHBaseAdmin().createTable(htd);
+    ExceptionInReseekRegionObserver.reset();
+    ExceptionInReseekRegionObserver.throwOnce.set(false); // throw exceptions in every retry
+    try (Table t = TEST_UTIL.getConnection().getTable(name)) {
+      TEST_UTIL.loadTable(t, FAMILY, false);
+      TEST_UTIL.getHBaseAdmin().flush(name);
+      countRows(t, new Scan().addColumn(FAMILY, FAMILY));
+      fail("Should have thrown an exception");
+    } catch (RuntimeException expected) {
+      // expected
+    }
+    assertTrue(ExceptionInReseekRegionObserver.reqCount.get() >= 3);
+  }
+
   /*
    * @param key
    * @return Scan with RowFilter that does LESS than passed key.


Mime
View raw message