Skip to content

HDDS-5248. SCM HA: Continuous PipelineNotFoundException seen in SCM log. #2267

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

Closed
lokeshj1703 wants to merge 4 commits into from
Closed

HDDS-5248. SCM HA: Continuous PipelineNotFoundException seen in SCM log. #2267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3be96d0
HDDS-5248. SCM HA: Continuous PipelineNotFoundException seen in SCM log.
lokeshj1703 May 19, 2021
95ae0d9
Make known exceptions subclass of SCMException
lokeshj1703 May 20, 2021
68f4ab0
Fix checkstyle issues
lokeshj1703 May 21, 2021
675beb8
Fix test failure
lokeshj1703 May 21, 2021
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
14 changes: 8 additions & 6 deletions ...op-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,21 @@

package org.apache.hadoop.hdds.scm.container;

import java.io.IOException;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;

/**
* Signals that ContainerException of some sort has occurred. This is parent
* of all the exceptions thrown by ContainerManager.
*/
public class ContainerException extends IOException {
public class ContainerException extends SCMException {

/**
* Constructs an {@code ContainerException} with {@code null}
* as its error detail message.
* @param resultCode ResultCode for the exception
*/
public ContainerException() {
super();
public ContainerException(ResultCodes resultCode) {
super(resultCode);
}

/**
Expand All @@ -39,8 +40,9 @@ public ContainerException() {
* @param message
* The detail message (which is saved for later retrieval
* by the {@link #getMessage()} method)
* @param resultCode ResultCode for the exception
*/
public ContainerException(String message) {
super(message);
public ContainerException(String message, ResultCodes resultCode) {
super(message, resultCode);
}
}
4 changes: 2 additions & 2 deletions ...common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerNotFoundException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class ContainerNotFoundException extends ContainerException {
* as its error detail message.
*/
public ContainerNotFoundException() {
super();
super(ResultCodes.CONTAINER_NOT_FOUND);
}

/**
Expand All @@ -39,6 +39,6 @@ public ContainerNotFoundException() {
* by the {@link #getMessage()} method)
*/
public ContainerNotFoundException(String message) {
super(message);
super(message, ResultCodes.CONTAINER_NOT_FOUND);
}
}
4 changes: 2 additions & 2 deletions ...src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaNotFoundException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class ContainerReplicaNotFoundException extends ContainerException {
* as its error detail message.
*/
public ContainerReplicaNotFoundException() {
super();
super(ResultCodes.CONTAINER_REPLICA_NOT_FOUND);
}

/**
Expand All @@ -40,6 +40,6 @@ public ContainerReplicaNotFoundException() {
* by the {@link #getMessage()} method)
*/
public ContainerReplicaNotFoundException(String message) {
super(message);
super(message, ResultCodes.CONTAINER_REPLICA_NOT_FOUND);
}
}
4 changes: 4 additions & 0 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/exceptions/SCMException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,5 +127,9 @@ public enum ResultCodes {
FAILED_TO_INIT_LEADER_CHOOSE_POLICY,
SCM_NOT_LEADER,
FAILED_TO_REVOKE_CERTIFICATES,
PIPELINE_NOT_FOUND,
UNKNOWN_PIPELINE_STATE,
CONTAINER_NOT_FOUND,
CONTAINER_REPLICA_NOT_FOUND
}
}
26 changes: 20 additions & 6 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.ratis.ServerNotLeaderException;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.server.ServerUtils;
import org.apache.hadoop.io.retry.RetryPolicy;
import org.apache.hadoop.ipc.RemoteException;
Expand Down Expand Up @@ -65,6 +66,13 @@ public final class SCMHAUtils {
.add(ResourceUnavailableException.class)
.build();

private static final List<Class<? extends Exception>>
NON_RETRIABLE_EXCEPTION_LIST =
ImmutableList.<Class<? extends Exception>>builder()
.add(SCMException.class)
.add(NonRetriableException.class)
.build();

private SCMHAUtils() {
// not used
}
Expand Down Expand Up @@ -206,13 +214,14 @@ public static Collection<String> getSCMNodeIds(
return getSCMNodeIds(configuration, scmServiceId);
}

private static Throwable unwrapException(Exception e) {
IOException ioException = null;
public static Throwable unwrapException(Exception e) {
Throwable cause = e.getCause();
if (cause instanceof RemoteException) {
ioException = ((RemoteException) cause).unwrapRemoteException();
if (e instanceof RemoteException) {
return ((RemoteException) e).unwrapRemoteException();
} else if (cause instanceof RemoteException) {
return ((RemoteException) cause).unwrapRemoteException();
}
return ioException == null ? e : ioException;
return e;
}

/**
Expand All @@ -231,7 +240,12 @@ public static boolean isNonRetriableException(Exception e) {
*/
public static boolean checkNonRetriableException(Exception e) {
Throwable t = unwrapException(e);
return NonRetriableException.class.isInstance(t);
for (Class<? extends Exception> clazz : NON_RETRIABLE_EXCEPTION_LIST) {
if (clazz.isInstance(t)) {
return true;
}
}
return false;
}

// This will return the underlying exception after unwrapping
Expand Down
8 changes: 4 additions & 4 deletions ...s/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineNotFoundException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@

package org.apache.hadoop.hdds.scm.pipeline;

import java.io.IOException;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;

/**
* Signals that a pipeline is missing from PipelineManager.
*/
public class PipelineNotFoundException extends IOException{
public class PipelineNotFoundException extends SCMException {
/**
* Constructs an {@code PipelineNotFoundException} with {@code null}
* as its error detail message.
*/
public PipelineNotFoundException() {
super();
super(ResultCodes.PIPELINE_NOT_FOUND);
Copy link
Contributor

@bharatviswa504 bharatviswa504 May 21, 2021

Choose a reason for hiding this comment

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

Looks like this ResultCode is lost when received at client.
But I see the message with whole stacktrace with debug.

Copy link
Contributor Author

@lokeshj1703 lokeshj1703 May 21, 2021

Choose a reason for hiding this comment

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

The latest commit adds the result code in the protobuf. Did you see that behavior in latest commit @bharatviswa504 ?

Copy link
Contributor

@bharatviswa504 bharatviswa504 May 21, 2021

Choose a reason for hiding this comment

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

Yes. I believe the ResultCodes in proto will help when we want to return ResultCodes in Response and construct the Exception based on that.

I believe when Rpc Layer constructs this Exception it used IOException default constructor and the ResultCode will be lost, and the message will be present.

Copy link
Contributor

@bharatviswa504 bharatviswa504 May 21, 2021

Choose a reason for hiding this comment

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

RemoteException unwrapException code and instantiateException, only message will be saved.

    try {
      Class<?> realClass = Class.forName(getClassName());
      return instantiateException(realClass.asSubclass(IOException.class));
    } catch(Exception e) {
      // cannot instantiate the original exception, just return this
    }
    return this;
  }

private IOException instantiateException(Class<? extends IOException> cls)
      throws Exception {
    Constructor<? extends IOException> cn = cls.getConstructor(String.class);
    cn.setAccessible(true);
    IOException ex = cn.newInstance(this.getMessage());
    ex.initCause(this);
    return ex;
  }

}

/**
Expand All @@ -41,6 +41,6 @@ public PipelineNotFoundException() {
* by the {@link #getMessage()} method)
*/
public PipelineNotFoundException(String message) {
super(message);
super(message, ResultCodes.PIPELINE_NOT_FOUND);
}
}
8 changes: 4 additions & 4 deletions ...mmon/src/main/java/org/apache/hadoop/hdds/scm/pipeline/UnknownPipelineStateException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@

package org.apache.hadoop.hdds.scm.pipeline;

import java.io.IOException;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;

/**
* Signals that a pipeline state is not recognized.
*/
public class UnknownPipelineStateException extends IOException {
public class UnknownPipelineStateException extends SCMException {
/**
* Constructs an {@code UnknownPipelineStateException} with {@code null}
* as its error detail message.
*/
public UnknownPipelineStateException() {
super();
super(ResultCodes.UNKNOWN_PIPELINE_STATE);
}

/**
Expand All @@ -41,6 +41,6 @@ public UnknownPipelineStateException() {
* by the {@link #getMessage()} method)
*/
public UnknownPipelineStateException(String message) {
super(message);
super(message, ResultCodes.UNKNOWN_PIPELINE_STATE);
}
}
4 changes: 4 additions & 0 deletions hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ enum Status {
FAILED_TO_INIT_LEADER_CHOOSE_POLICY = 31;
SCM_NOT_LEADER = 32;
FAILED_TO_REVOKE_CERTIFICATES = 33;
PIPELINE_NOT_FOUND = 34;
UNKNOWN_PIPELINE_STATE = 35;
CONTAINER_NOT_FOUND = 36;
CONTAINER_REPLICA_NOT_FOUND = 37;
}

/**
Expand Down
21 changes: 20 additions & 1 deletion ...ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
import org.apache.hadoop.hdds.scm.client.ScmClient;
import org.apache.hadoop.hdds.scm.ha.SCMHAUtils;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

import org.junit.Rule;
import org.junit.rules.Timeout;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -80,4 +83,20 @@ public void testCreate() throws Exception {
.getContainerID());
}

/**
* A simple test to get Pipeline with {@link ContainerOperationClient}.
* @throws Exception
*/
@Test
public void testGetPipeline() throws Exception {
try {
storageClient.getPipeline(PipelineID.randomId().getProtobuf());
Assert.fail("Get Pipeline should fail");
} catch (Exception e) {
Assert.assertTrue(
SCMHAUtils.unwrapException(e) instanceof PipelineNotFoundException);
}

Assert.assertFalse(storageClient.listPipelines().isEmpty());
}
}