-
Notifications
You must be signed in to change notification settings - Fork 594
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe to properly handle all the scenarios, in the error scenario(Which are SCM specific errors), we should return the response with Status false and other info as needed, and not throw an exception.
|
@bharatviswa504 @bshashikant As per discussion, I have updated the PR. Some APIs throw IOException directly. Those APIs will still need to be handled separately. |
bharatviswa504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM.
Can we open a Jira to handle IOException related.
| */ | ||
| public PipelineNotFoundException() { | ||
| super(); | ||
| super(ResultCodes.PIPELINE_NOT_FOUND); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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;
}
|
HDDS-5259 has been created to handle the IOException being thrown by the apis in StorageContainerLocationProtocol. |
|
@bharatviswa504 Thanks for the review! I have committed the PR to master branch. |
…og. (apache#2267) (cherry picked from commit 9080fc3) Change-Id: I30245c1c2742676aad00776f81a328f3d75c7f43
What changes were proposed in this pull request?
After running decommissioning tests and aborting few decommissioning command, seeing PipelineNotFoundException INFO logs continuously in scm log.
Although these are INFO logs, raising the issue as the exceptions are thrown in loop.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5248
How was this patch tested?
Adds UT