Skip to content

HBASE-22699 refactor isMetaClearingException #501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2019
Merged

HBASE-22699 refactor isMetaClearingException #501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocatorHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import static org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil.findException;
import static org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil.isMetaClearingException;
import static org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil.isRegionServerOverloadedException;

import java.util.Arrays;
import java.util.Optional;
Expand Down Expand Up @@ -67,7 +68,8 @@ static void updateCachedLocationOnError(HRegionLocation loc, Throwable exception
LOG.debug("The actual exception when updating {} is {}", loc,
cause != null ? cause.toString() : "none");
}
if (cause == null || !isMetaClearingException(cause)) {
if (cause == null || !isMetaClearingException(cause)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now we need to modify the code other than ClientExceptionsUtil, this means we need to create patch for branch-2? As on branch-2, I believe there are other code paths which need to handle this...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I create another pull request to branch2: #578

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnhomsea You created a new PR. Does it subsume this one? Or is it complementary? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it subsumes this one, but branch-2 is different from master in some cases.

|| isRegionServerOverloadedException(cause)) {
LOG.debug("Will not update {} because the exception is null or not the one we care about",
loc);
return;
Expand Down
31 changes: 20 additions & 11 deletions hbase-client/src/main/java/org/apache/hadoop/hbase/exceptions/ClientExceptionsUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.hadoop.hbase.CallDroppedException;
import org.apache.hadoop.hbase.CallQueueTooBigException;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.MultiActionResultTooLarge;
import org.apache.hadoop.hbase.NotServingRegionException;
import org.apache.hadoop.hbase.RegionTooBusyException;
import org.apache.hadoop.hbase.RetryImmediatelyException;
Expand All @@ -59,18 +58,28 @@ public static boolean isMetaClearingException(Throwable cur) {
if (cur == null) {
return true;
}
return !isSpecialException(cur) || (cur instanceof RegionMovedException)
|| cur instanceof NotServingRegionException;
return !isMetaCachePreservingException(cur);
}

public static boolean isSpecialException(Throwable cur) {
return (cur instanceof RegionMovedException || cur instanceof RegionOpeningException
|| cur instanceof RegionTooBusyException || cur instanceof RpcThrottlingException
|| cur instanceof MultiActionResultTooLarge || cur instanceof RetryImmediatelyException
|| cur instanceof CallQueueTooBigException || cur instanceof CallDroppedException
|| cur instanceof NotServingRegionException || cur instanceof RequestTooBigException);
public static boolean isRegionServerOverloadedException(Throwable t) {
t = findException(t);
return isInstanceOfRegionServerOverloadedException(t);
}

private static boolean isInstanceOfRegionServerOverloadedException(Throwable t) {
return t instanceof CallQueueTooBigException || t instanceof CallDroppedException;
}

private static boolean isMetaCachePreservingException(Throwable t) {
return t instanceof RegionOpeningException || t instanceof RegionTooBusyException
|| t instanceof RpcThrottlingException || t instanceof RetryImmediatelyException
|| t instanceof RequestTooBigException;
}

private static boolean isExceptionWeCare(Throwable t) {
return isMetaCachePreservingException(t) || isInstanceOfRegionServerOverloadedException(t)
|| t instanceof NotServingRegionException;
}

/**
* Look for an exception we know in the remote exception:
Expand All @@ -87,15 +96,15 @@ public static Throwable findException(Object exception) {
}
Throwable cur = (Throwable) exception;
while (cur != null) {
if (isSpecialException(cur)) {
if (isExceptionWeCare(cur)) {
return cur;
}
if (cur instanceof RemoteException) {
RemoteException re = (RemoteException) cur;
cur = re.unwrapRemoteException();

// unwrapRemoteException can return the exception given as a parameter when it cannot
// unwrap it. In this case, there is no need to look further
// unwrap it. In this case, there is no need to look further
// noinspection ObjectEquality
if (cur == re) {
return cur;
Expand Down
11 changes: 8 additions & 3 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Arrays;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.CallDroppedException;
import org.apache.hadoop.hbase.CallQueueTooBigException;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
Expand All @@ -38,6 +39,7 @@
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil;
import org.apache.hadoop.hbase.exceptions.RegionOpeningException;
import org.apache.hadoop.hbase.exceptions.RequestTooBigException;
import org.apache.hadoop.hbase.quotas.RpcThrottlingException;
import org.apache.hadoop.hbase.regionserver.HRegionServer;
import org.apache.hadoop.hbase.regionserver.RSRpcServices;
Expand Down Expand Up @@ -149,12 +151,14 @@ public void testPreserveMetaCacheOnException() throws Exception {
table.mutateRow(mutations);
} catch (IOException ex) {
// Only keep track of the last exception that updated the meta cache
if (ClientExceptionsUtil.isMetaClearingException(ex) || success) {
if ((ClientExceptionsUtil.isMetaClearingException(ex)
&& !ClientExceptionsUtil.isRegionServerOverloadedException(ex)) || success) {
exp = ex;
}
}
// Do not test if we did not touch the meta cache in this iteration.
if (exp != null && ClientExceptionsUtil.isMetaClearingException(exp)) {
if (exp != null && ClientExceptionsUtil.isMetaClearingException(exp)
&& !ClientExceptionsUtil.isRegionServerOverloadedException(exp)) {
assertNull(locator.getRegionLocationInCache(TABLE_NAME, row));
} else if (success) {
assertNotNull(locator.getRegionLocationInCache(TABLE_NAME, row));
Expand Down Expand Up @@ -199,7 +203,8 @@ public static List<Throwable> metaCachePreservingExceptions() {
return Arrays.asList(new RegionOpeningException(" "),
new RegionTooBusyException("Some old message"), new RpcThrottlingException(" "),
new MultiActionResultTooLarge(" "), new RetryImmediatelyException(" "),
new CallQueueTooBigException());
new RequestTooBigException(), new CallQueueTooBigException(),
new CallDroppedException());
}

public static class RegionServerWithFakeRpcServices extends HRegionServer {
Expand Down