chihsuan commented on code in PR #11763:
URL: https://github.com/apache/ozone/pull/11763#discussion_r4430868105
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java:
##########
@@ -108,10 +109,8 @@ public void submitRequest(OMRequest request,
responseObserver.onNext(omResponse);
} catch (Throwable e) {
LOG.error("Failed to submit request", e);
- IOException ex = new IOException(e.getCause());
- responseObserver.onError(
- Status.INTERNAL.withDescription(ex.getMessage())
- .asRuntimeException());
+ Throwable omException = e instanceof ServiceException ? e.getCause() : e;
Review Comment:
nit: probably not reachable today, but if a ServiceException has no cause,
toProto may throw NPE. Maybe fall back to e here, just to be defensive?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java:
##########
@@ -226,13 +226,9 @@ public RetryAction shouldRetry(Exception exception, int
retries,
if (notLeaderException != null) {
// Prepare the next OM to be tried. This will help with calculation
// of the wait times needed get creating the retryAction.
- String suggestedLeaderAddress =
- notLeaderException.getSuggestedLeaderAddress();
String suggestedNodeId =
notLeaderException.getSuggestedLeaderNodeId();
- if (suggestedLeaderAddress != null &&
- suggestedNodeId != null &&
- omProxies.contains(suggestedNodeId, suggestedLeaderAddress)) {
+ if (suggestedNodeId != null && omProxies.get(suggestedNodeId) !=
null) {
Review Comment:
I might be missing something, but if the suggested leader is down, the
client seems to bounce between two OMs with no wait. Would it make sense to
back off on repeated suggestions?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java:
##########
@@ -243,30 +243,21 @@ private OMResponse submitRequestToHost(OMRequest payload,
String targetHost)
}
public static Exception unwrapException(Exception ex) {
- Exception grpcException = null;
- try {
- StatusRuntimeException srexp =
- (StatusRuntimeException)ex.getCause();
- Status status = srexp.getStatus();
- LOG.debug("GRPC exception wrapped: {}", status.getDescription());
- if (status.getCode() == Status.Code.INTERNAL) {
- // exception potentially generated by OzoneManagerServiceGrpc
- String description = status.getDescription();
- int colonIndex = description.indexOf(':');
- grpcException = new ServiceException(new
RemoteException(description.substring(0, colonIndex),
- description.substring(colonIndex + 2)));
- } else if ((status.getCode() == Status.Code.RESOURCE_EXHAUSTED) ||
- (status.getCode() == Status.Code.DATA_LOSS)) {
- grpcException = srexp;
- } else {
- // exception generated by connection failure, gRPC
- grpcException = ex;
+ StatusRuntimeException srexp = (StatusRuntimeException) ex.getCause();
+ Status status = srexp.getStatus();
+ LOG.debug("GRPC exception wrapped: {}", status.getDescription());
+ if (status.getCode() == Status.Code.INTERNAL) {
+ IOException omException = OMGrpcError.toOMException(srexp);
+ if (omException != null) {
+ // exception generated by OzoneManagerServiceGrpc
+ return new ServiceException(omException);
}
- } catch (Exception e) {
- grpcException = new IOException(e);
- LOG.error("error unwrapping exception from OMResponse {}");
+ } else if ((status.getCode() == Status.Code.RESOURCE_EXHAUSTED) ||
+ (status.getCode() == Status.Code.DATA_LOSS)) {
+ return srexp;
}
- return grpcException;
+ // exception generated by gRPC, e.g. a connection failure
+ return ex;
Review Comment:
Just to confirm, do we need to handle a new S3G with an older OM? Without
the trailer, NotLeader and LeaderNotReady become plain errors here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
