From ac3040fb58c9a95aea4824e3ba77019698afddc4 Mon Sep 17 00:00:00 2001 From: sdeka Date: Thu, 10 Oct 2019 15:28:17 +0530 Subject: [PATCH 1/5] HDDS-2206. Separate handling for OMException and IOException in the Ozone Manager. --- .../src/main/resources/ozone-default.xml | 14 ++++++++ .../apache/hadoop/ozone/om/OMConfigKeys.java | 7 ++++ .../apache/hadoop/ozone/om/OzoneManager.java | 16 ++++----- ...ManagerProtocolServerSideTranslatorPB.java | 35 ++++++++++++++++--- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index b0a59fa209cc..3927f5d86d40 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -1652,6 +1652,20 @@ + + ozone.om.exception.stacktrace.propagate + true + OZONE, OM, DEBUG + + If true, propagate full stacktrace for system exceptions to the client. + If false, propagate summary message only and log stacktrace on server. + Full stacktrace on the client is useful for developers to debug + unexpected system errors encountered on the server while handling a + request. + This setting is not applicable for business exceptions. + + + ozone.om.ratis.client.request.timeout.duration 3s diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index dcb9b5cdeac9..100c95567824 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -151,6 +151,13 @@ private OMConfigKeys() { public static final TimeDuration OZONE_OM_RATIS_MINIMUM_TIMEOUT_DEFAULT = TimeDuration.valueOf(1, TimeUnit.SECONDS); + // Propagate stack trace for System Exceptions to the client. + public static final String OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE + = "ozone.om.exception.stacktrace.propagate"; + public static final boolean + OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT + = true; + // OM Ratis client configurations public static final String OZONE_OM_RATIS_CLIENT_REQUEST_TIMEOUT_DURATION_KEY = "ozone.om.ratis.client.request.timeout.duration"; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 0cd087eee236..479953d4f47e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -200,15 +200,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_FILE; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_TEMP_FILE; import static org.apache.hadoop.ozone.OzoneConsts.RPC_PORT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.*; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_AUTH_METHOD; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; @@ -1195,8 +1187,12 @@ private RPC.Server getRpcServer(OzoneConfiguration conf) throws IOException { OZONE_OM_HANDLER_COUNT_DEFAULT); RPC.setProtocolEngine(configuration, OzoneManagerProtocolPB.class, ProtobufRpcEngine.class); + final boolean propagateExceptionStack = + conf.getBoolean(OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, + OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); this.omServerProtocol = new OzoneManagerProtocolServerSideTranslatorPB( - this, omRatisServer, omClientProtocolMetrics, isRatisEnabled); + this, omRatisServer, omClientProtocolMetrics, isRatisEnabled, + propagateExceptionStack); BlockingService omService = newReflectiveBlockingService(omServerProtocol); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index ff2c966983f4..483b03de55c7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,6 +22,7 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.exceptions.NotLeaderException; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolPB; import org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer; import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer; @@ -56,6 +57,7 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements private final OzoneManagerRatisServer omRatisServer; private final RequestHandler handler; private final boolean isRatisEnabled; + private final boolean propagateExceptionStack; private final OzoneManager ozoneManager; private final OzoneManagerDoubleBuffer ozoneManagerDoubleBuffer; private final AtomicLong transactionIndex = new AtomicLong(0L); @@ -71,11 +73,13 @@ public OzoneManagerProtocolServerSideTranslatorPB( OzoneManager impl, OzoneManagerRatisServer ratisServer, ProtocolMessageMetrics metrics, - boolean enableRatis) { + boolean enableRatis, + boolean propagateExceptionStack) { this.ozoneManager = impl; handler = new OzoneManagerRequestHandler(impl); this.omRatisServer = ratisServer; this.isRatisEnabled = enableRatis; + this.propagateExceptionStack = propagateExceptionStack; this.ozoneManagerDoubleBuffer = new OzoneManagerDoubleBuffer(ozoneManager.getMetadataManager(), (i) -> { // Do nothing. @@ -118,7 +122,7 @@ private OMResponse processRequest(OMRequest request) throws request = omClientRequest.preExecute(ozoneManager); } catch (IOException ex) { // As some of the preExecute returns error. So handle here. - return createErrorResponse(request, ex); + return failRequestWithException(request, ex); } return submitRequestToRatis(request); } else { @@ -131,7 +135,27 @@ private OMResponse processRequest(OMRequest request) throws } } } else { - return submitRequestDirectlyToOM(request); + try { + OMResponse response = submitRequestDirectlyToOM(request); + return response; + } catch (IOException ex) { + return failRequestWithException(request, ex); + } + } + } + + private OMResponse failRequestWithException(OMRequest request, IOException e) + throws ServiceException { + // send summary error response for business exceptions. + if (e instanceof OMException) { + return createErrorResponse(request, e); + } + // propagate full stack trace for System Exceptions? + if (propagateExceptionStack) { + throw new ServiceException(e.getMessage(), e); + } else { + LOG.error(e.getMessage(), e); + return createErrorResponse(request, e); } } @@ -202,7 +226,8 @@ private ServiceException createNotLeaderException() { /** * Submits request directly to OM. */ - private OMResponse submitRequestDirectlyToOM(OMRequest request) { + private OMResponse submitRequestDirectlyToOM(OMRequest request) + throws IOException { OMClientResponse omClientResponse = null; long index = 0L; try { @@ -219,7 +244,7 @@ private OMResponse submitRequestDirectlyToOM(OMRequest request) { omClientResponse = omClientRequest.validateAndUpdateCache( ozoneManager, index, ozoneManagerDoubleBuffer::add); } - } catch(IOException ex) { + } catch(OMException ex) { // As some of the preExecute returns error. So handle here. return createErrorResponse(request, ex); } From 5fff037ed185d8f6dca4078d8625bdb2ac68be92 Mon Sep 17 00:00:00 2001 From: sdeka Date: Tue, 15 Oct 2019 15:31:05 +0530 Subject: [PATCH 2/5] addressed review comments. --- .../apache/hadoop/ozone/OzoneConfigKeys.java | 4 +++ .../apache/hadoop/ozone/om/OMConfigKeys.java | 7 ----- .../apache/hadoop/ozone/om/OzoneManager.java | 18 +++++++++-- ...ManagerProtocolServerSideTranslatorPB.java | 30 ++++++++----------- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index 3f7d0b915d5d..b0e1d4bb5b37 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -80,6 +80,10 @@ public final class OzoneConfigKeys { public static final String OZONE_TRACE_ENABLED_KEY = "ozone.trace.enabled"; public static final boolean OZONE_TRACE_ENABLED_DEFAULT = false; + public static final String OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE = + "ozone.exception.stacktrace.propagate"; + public static final boolean + OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT = true; public static final String OZONE_METADATA_STORE_IMPL = "ozone.metastore.impl"; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 100c95567824..dcb9b5cdeac9 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -151,13 +151,6 @@ private OMConfigKeys() { public static final TimeDuration OZONE_OM_RATIS_MINIMUM_TIMEOUT_DEFAULT = TimeDuration.valueOf(1, TimeUnit.SECONDS); - // Propagate stack trace for System Exceptions to the client. - public static final String OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE - = "ozone.om.exception.stacktrace.propagate"; - public static final boolean - OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT - = true; - // OM Ratis client configurations public static final String OZONE_OM_RATIS_CLIENT_REQUEST_TIMEOUT_DURATION_KEY = "ozone.om.ratis.client.request.timeout.duration"; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 479953d4f47e..5513df544db4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -197,10 +197,22 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX_DEFAULT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT; +import static org.apache.hadoop.ozone.OzoneConfigKeys + .OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE; +import static org.apache.hadoop.ozone.OzoneConfigKeys + .OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_FILE; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_TEMP_FILE; import static org.apache.hadoop.ozone.OzoneConsts.RPC_PORT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.*; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_AUTH_METHOD; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; @@ -1188,8 +1200,8 @@ private RPC.Server getRpcServer(OzoneConfiguration conf) throws IOException { RPC.setProtocolEngine(configuration, OzoneManagerProtocolPB.class, ProtobufRpcEngine.class); final boolean propagateExceptionStack = - conf.getBoolean(OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, - OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); + conf.getBoolean(OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, + OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); this.omServerProtocol = new OzoneManagerProtocolServerSideTranslatorPB( this, omRatisServer, omClientProtocolMetrics, isRatisEnabled, propagateExceptionStack); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 483b03de55c7..8e1645f976b0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -230,24 +230,20 @@ private OMResponse submitRequestDirectlyToOM(OMRequest request) throws IOException { OMClientResponse omClientResponse = null; long index = 0L; - try { - if (OmUtils.isReadOnly(request)) { - return handler.handle(request); - } else { - OMClientRequest omClientRequest = - OzoneManagerRatisUtils.createClientRequest(request); - Preconditions.checkState(omClientRequest != null, - "Unrecognized write command type request" + request.toString()); - request = omClientRequest.preExecute(ozoneManager); - index = transactionIndex.incrementAndGet(); - omClientRequest = OzoneManagerRatisUtils.createClientRequest(request); - omClientResponse = omClientRequest.validateAndUpdateCache( - ozoneManager, index, ozoneManagerDoubleBuffer::add); - } - } catch(OMException ex) { - // As some of the preExecute returns error. So handle here. - return createErrorResponse(request, ex); + if (OmUtils.isReadOnly(request)) { + return handler.handle(request); + } else { + OMClientRequest omClientRequest = + OzoneManagerRatisUtils.createClientRequest(request); + Preconditions.checkState(omClientRequest != null, + "Unrecognized write command type request" + request.toString()); + request = omClientRequest.preExecute(ozoneManager); + index = transactionIndex.incrementAndGet(); + omClientRequest = OzoneManagerRatisUtils.createClientRequest(request); + omClientResponse = omClientRequest.validateAndUpdateCache( + ozoneManager, index, ozoneManagerDoubleBuffer::add); } + try { omClientResponse.getFlushFuture().get(); if (LOG.isTraceEnabled()) { From 9a1f441905b567bd41e3ffa668444f08cc8f752a Mon Sep 17 00:00:00 2001 From: sdeka Date: Thu, 17 Oct 2019 14:59:57 +0530 Subject: [PATCH 3/5] Revert "addressed review comments." This reverts commit 5fff037ed185d8f6dca4078d8625bdb2ac68be92. --- .../apache/hadoop/ozone/OzoneConfigKeys.java | 4 --- .../apache/hadoop/ozone/om/OMConfigKeys.java | 7 +++++ .../apache/hadoop/ozone/om/OzoneManager.java | 18 ++--------- ...ManagerProtocolServerSideTranslatorPB.java | 30 +++++++++++-------- 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index b0e1d4bb5b37..3f7d0b915d5d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -80,10 +80,6 @@ public final class OzoneConfigKeys { public static final String OZONE_TRACE_ENABLED_KEY = "ozone.trace.enabled"; public static final boolean OZONE_TRACE_ENABLED_DEFAULT = false; - public static final String OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE = - "ozone.exception.stacktrace.propagate"; - public static final boolean - OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT = true; public static final String OZONE_METADATA_STORE_IMPL = "ozone.metastore.impl"; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index dcb9b5cdeac9..100c95567824 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -151,6 +151,13 @@ private OMConfigKeys() { public static final TimeDuration OZONE_OM_RATIS_MINIMUM_TIMEOUT_DEFAULT = TimeDuration.valueOf(1, TimeUnit.SECONDS); + // Propagate stack trace for System Exceptions to the client. + public static final String OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE + = "ozone.om.exception.stacktrace.propagate"; + public static final boolean + OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT + = true; + // OM Ratis client configurations public static final String OZONE_OM_RATIS_CLIENT_REQUEST_TIMEOUT_DURATION_KEY = "ozone.om.ratis.client.request.timeout.duration"; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 5513df544db4..479953d4f47e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -197,22 +197,10 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX_DEFAULT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT; -import static org.apache.hadoop.ozone.OzoneConfigKeys - .OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE; -import static org.apache.hadoop.ozone.OzoneConfigKeys - .OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_FILE; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_TEMP_FILE; import static org.apache.hadoop.ozone.OzoneConsts.RPC_PORT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.*; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_AUTH_METHOD; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; @@ -1200,8 +1188,8 @@ private RPC.Server getRpcServer(OzoneConfiguration conf) throws IOException { RPC.setProtocolEngine(configuration, OzoneManagerProtocolPB.class, ProtobufRpcEngine.class); final boolean propagateExceptionStack = - conf.getBoolean(OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, - OZONE_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); + conf.getBoolean(OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, + OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); this.omServerProtocol = new OzoneManagerProtocolServerSideTranslatorPB( this, omRatisServer, omClientProtocolMetrics, isRatisEnabled, propagateExceptionStack); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 8e1645f976b0..483b03de55c7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -230,20 +230,24 @@ private OMResponse submitRequestDirectlyToOM(OMRequest request) throws IOException { OMClientResponse omClientResponse = null; long index = 0L; - if (OmUtils.isReadOnly(request)) { - return handler.handle(request); - } else { - OMClientRequest omClientRequest = - OzoneManagerRatisUtils.createClientRequest(request); - Preconditions.checkState(omClientRequest != null, - "Unrecognized write command type request" + request.toString()); - request = omClientRequest.preExecute(ozoneManager); - index = transactionIndex.incrementAndGet(); - omClientRequest = OzoneManagerRatisUtils.createClientRequest(request); - omClientResponse = omClientRequest.validateAndUpdateCache( - ozoneManager, index, ozoneManagerDoubleBuffer::add); + try { + if (OmUtils.isReadOnly(request)) { + return handler.handle(request); + } else { + OMClientRequest omClientRequest = + OzoneManagerRatisUtils.createClientRequest(request); + Preconditions.checkState(omClientRequest != null, + "Unrecognized write command type request" + request.toString()); + request = omClientRequest.preExecute(ozoneManager); + index = transactionIndex.incrementAndGet(); + omClientRequest = OzoneManagerRatisUtils.createClientRequest(request); + omClientResponse = omClientRequest.validateAndUpdateCache( + ozoneManager, index, ozoneManagerDoubleBuffer::add); + } + } catch(OMException ex) { + // As some of the preExecute returns error. So handle here. + return createErrorResponse(request, ex); } - try { omClientResponse.getFlushFuture().get(); if (LOG.isTraceEnabled()) { From 4e65849ae6cd793830201c44e578c0ce04355543 Mon Sep 17 00:00:00 2001 From: sdeka Date: Thu, 17 Oct 2019 15:21:57 +0530 Subject: [PATCH 4/5] Revert "HDDS-2206. Separate handling for OMException and IOException in the Ozone Manager." This reverts commit ac3040fb58c9a95aea4824e3ba77019698afddc4. --- .../src/main/resources/ozone-default.xml | 14 -------- .../apache/hadoop/ozone/om/OMConfigKeys.java | 7 ---- .../apache/hadoop/ozone/om/OzoneManager.java | 16 +++++---- ...ManagerProtocolServerSideTranslatorPB.java | 35 +++---------------- 4 files changed, 15 insertions(+), 57 deletions(-) diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 3927f5d86d40..b0a59fa209cc 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -1652,20 +1652,6 @@ - - ozone.om.exception.stacktrace.propagate - true - OZONE, OM, DEBUG - - If true, propagate full stacktrace for system exceptions to the client. - If false, propagate summary message only and log stacktrace on server. - Full stacktrace on the client is useful for developers to debug - unexpected system errors encountered on the server while handling a - request. - This setting is not applicable for business exceptions. - - - ozone.om.ratis.client.request.timeout.duration 3s diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 100c95567824..dcb9b5cdeac9 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -151,13 +151,6 @@ private OMConfigKeys() { public static final TimeDuration OZONE_OM_RATIS_MINIMUM_TIMEOUT_DEFAULT = TimeDuration.valueOf(1, TimeUnit.SECONDS); - // Propagate stack trace for System Exceptions to the client. - public static final String OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE - = "ozone.om.exception.stacktrace.propagate"; - public static final boolean - OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT - = true; - // OM Ratis client configurations public static final String OZONE_OM_RATIS_CLIENT_REQUEST_TIMEOUT_DURATION_KEY = "ozone.om.ratis.client.request.timeout.duration"; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 479953d4f47e..0cd087eee236 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -200,7 +200,15 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_FILE; import static org.apache.hadoop.ozone.OzoneConsts.OM_METRICS_TEMP_FILE; import static org.apache.hadoop.ozone.OzoneConsts.RPC_PORT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.*; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HANDLER_COUNT_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_METRICS_SAVE_INTERVAL_DEFAULT; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_AUTH_METHOD; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; @@ -1187,12 +1195,8 @@ private RPC.Server getRpcServer(OzoneConfiguration conf) throws IOException { OZONE_OM_HANDLER_COUNT_DEFAULT); RPC.setProtocolEngine(configuration, OzoneManagerProtocolPB.class, ProtobufRpcEngine.class); - final boolean propagateExceptionStack = - conf.getBoolean(OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE, - OZONE_OM_PROPAGATE_SYSTEM_EXCEPTION_STACKTRACE_DEFAULT); this.omServerProtocol = new OzoneManagerProtocolServerSideTranslatorPB( - this, omRatisServer, omClientProtocolMetrics, isRatisEnabled, - propagateExceptionStack); + this, omRatisServer, omClientProtocolMetrics, isRatisEnabled); BlockingService omService = newReflectiveBlockingService(omServerProtocol); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 483b03de55c7..ff2c966983f4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,7 +22,6 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.exceptions.NotLeaderException; -import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolPB; import org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer; import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer; @@ -57,7 +56,6 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements private final OzoneManagerRatisServer omRatisServer; private final RequestHandler handler; private final boolean isRatisEnabled; - private final boolean propagateExceptionStack; private final OzoneManager ozoneManager; private final OzoneManagerDoubleBuffer ozoneManagerDoubleBuffer; private final AtomicLong transactionIndex = new AtomicLong(0L); @@ -73,13 +71,11 @@ public OzoneManagerProtocolServerSideTranslatorPB( OzoneManager impl, OzoneManagerRatisServer ratisServer, ProtocolMessageMetrics metrics, - boolean enableRatis, - boolean propagateExceptionStack) { + boolean enableRatis) { this.ozoneManager = impl; handler = new OzoneManagerRequestHandler(impl); this.omRatisServer = ratisServer; this.isRatisEnabled = enableRatis; - this.propagateExceptionStack = propagateExceptionStack; this.ozoneManagerDoubleBuffer = new OzoneManagerDoubleBuffer(ozoneManager.getMetadataManager(), (i) -> { // Do nothing. @@ -122,7 +118,7 @@ private OMResponse processRequest(OMRequest request) throws request = omClientRequest.preExecute(ozoneManager); } catch (IOException ex) { // As some of the preExecute returns error. So handle here. - return failRequestWithException(request, ex); + return createErrorResponse(request, ex); } return submitRequestToRatis(request); } else { @@ -135,27 +131,7 @@ private OMResponse processRequest(OMRequest request) throws } } } else { - try { - OMResponse response = submitRequestDirectlyToOM(request); - return response; - } catch (IOException ex) { - return failRequestWithException(request, ex); - } - } - } - - private OMResponse failRequestWithException(OMRequest request, IOException e) - throws ServiceException { - // send summary error response for business exceptions. - if (e instanceof OMException) { - return createErrorResponse(request, e); - } - // propagate full stack trace for System Exceptions? - if (propagateExceptionStack) { - throw new ServiceException(e.getMessage(), e); - } else { - LOG.error(e.getMessage(), e); - return createErrorResponse(request, e); + return submitRequestDirectlyToOM(request); } } @@ -226,8 +202,7 @@ private ServiceException createNotLeaderException() { /** * Submits request directly to OM. */ - private OMResponse submitRequestDirectlyToOM(OMRequest request) - throws IOException { + private OMResponse submitRequestDirectlyToOM(OMRequest request) { OMClientResponse omClientResponse = null; long index = 0L; try { @@ -244,7 +219,7 @@ private OMResponse submitRequestDirectlyToOM(OMRequest request) omClientResponse = omClientRequest.validateAndUpdateCache( ozoneManager, index, ozoneManagerDoubleBuffer::add); } - } catch(OMException ex) { + } catch(IOException ex) { // As some of the preExecute returns error. So handle here. return createErrorResponse(request, ex); } From 1d9dc03e6da25a26ac24be169169d2de87ba8ce6 Mon Sep 17 00:00:00 2001 From: sdeka Date: Thu, 17 Oct 2019 15:53:05 +0530 Subject: [PATCH 5/5] removed config. propagate stacktrace inside error message. --- ...zoneManagerProtocolServerSideTranslatorPB.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index ff2c966983f4..4dab8714a367 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,6 +22,7 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.exceptions.NotLeaderException; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolPB; import org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer; import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer; @@ -34,6 +35,7 @@ import com.google.protobuf.RpcController; import com.google.protobuf.ServiceException; +import org.apache.hadoop.util.StringUtils; import org.apache.ratis.protocol.RaftPeerId; import org.apache.ratis.util.ExitUtils; import org.slf4j.Logger; @@ -153,12 +155,21 @@ private OMResponse createErrorResponse( OzoneManagerRatisUtils.exceptionToResponseStatus(exception)) .setCmdType(cmdType) .setSuccess(false); - if (exception.getMessage() != null) { - omResponse.setMessage(exception.getMessage()); + String errorMsg = exceptionErrorMessage(exception); + if (errorMsg != null) { + omResponse.setMessage(errorMsg); } return omResponse.build(); } + private String exceptionErrorMessage(IOException ex) { + if (ex instanceof OMException) { + return ex.getMessage(); + } else { + return StringUtils.stringifyException(ex); + } + } + /** * Submits request to OM's Ratis server. */