You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by en...@apache.org on 2017/02/07 00:21:34 UTC
[2/2] hbase git commit: HBASE-17187 DoNotRetryExceptions from
coprocessors should bubble up to the application
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/62f7b395
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/62f7b395
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/62f7b395
Branch: refs/heads/branch-1.1
Commit: 62f7b395279301999b374392a2d9824ae191485f
Parents: 4307886
Author: Enis Soztutar <en...@apache.org>
Authored: Mon Feb 6 12:00:05 2017 -0800
Committer: Enis Soztutar <en...@apache.org>
Committed: Mon Feb 6 16:14:21 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/62f7b395/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 4a5e635..2c06fd5 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/62f7b395/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 fed9e62..6845a92 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
@@ -2540,7 +2540,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/62f7b395/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 aa8936c..224541f 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;
@@ -600,6 +603,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 {
@@ -615,8 +627,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);
}
@@ -650,6 +667,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);
@@ -659,6 +678,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.