This is an automated email from the ASF dual-hosted git repository.
okumin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new cf1d6d8670c HIVE-29563: Iceberg REST Catalog's error logs are too
verbose (#6425)
cf1d6d8670c is described below
commit cf1d6d8670c6b1da871b074099dc7ccd4b8257cd
Author: Shohei Okumiya <[email protected]>
AuthorDate: Wed May 6 21:44:42 2026 +0900
HIVE-29563: Iceberg REST Catalog's error logs are too verbose (#6425)
* HIVE-29563: Iceberg REST Catalog's error logs are too verbose
* Remove extra abstraction
* Stop sending stacktrace on client errors
* Log on 5xx
---
.../org/apache/iceberg/rest/HMSCatalogAdapter.java | 82 +++++-----------------
.../org/apache/iceberg/rest/HMSCatalogServlet.java | 16 +----
2 files changed, 17 insertions(+), 81 deletions(-)
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
index 252bde85a5e..8a9ea142d72 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
@@ -21,12 +21,13 @@
import com.google.common.base.Preconditions;
+import java.io.Closeable;
import java.io.IOException;
import java.time.Clock;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
-import java.util.function.Consumer;
+import javax.servlet.http.HttpServletResponse;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.BaseTransaction;
import org.apache.iceberg.Table;
@@ -48,7 +49,6 @@
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.exceptions.NotAuthorizedException;
-import org.apache.iceberg.exceptions.RESTException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.base.Splitter;
@@ -83,7 +83,7 @@
* Original @ <a
href="https://github.com/apache/iceberg/blob/apache-iceberg-1.9.1/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java";>RESTCatalogAdapter.java</a>
* Adaptor class to translate REST requests into {@link Catalog} API calls.
*/
-public class HMSCatalogAdapter implements RESTClient {
+public class HMSCatalogAdapter implements Closeable {
private static final Logger LOG =
LoggerFactory.getLogger(HMSCatalogAdapter.class);
private static final Splitter SLASH = Splitter.on('/');
@@ -506,7 +506,7 @@ <T extends RESTResponse> T execute(
String path,
Map<String, String> queryParams,
Object body,
- Consumer<ErrorResponse> errorHandler) {
+ HttpServletResponse response) throws IOException {
ErrorResponse.Builder errorBuilder = ErrorResponse.builder();
Pair<Route, Map<String, String>> routeAndVars = Route.from(method, path);
if (routeAndVars != null) {
@@ -527,63 +527,9 @@ <T extends RESTResponse> T execute(
.withMessage(String.format("No route for request: %s %s", method,
path));
}
ErrorResponse error = errorBuilder.build();
- errorHandler.accept(error);
- // if the error handler doesn't throw an exception, throw a generic one
- throw new RESTException("Unhandled error: %s", error);
- }
-
- @Override
- public <T extends RESTResponse> T delete(
- String path,
- Class<T> responseType,
- Map<String, String> headers,
- Consumer<ErrorResponse> errorHandler) {
- return execute(HTTPMethod.DELETE, path, null, null, errorHandler);
- }
-
- @Override
- public <T extends RESTResponse> T delete(
- String path,
- Map<String, String> queryParams,
- Class<T> responseType,
- Map<String, String> headers,
- Consumer<ErrorResponse> errorHandler) {
- return execute(HTTPMethod.DELETE, path, queryParams, null, errorHandler);
- }
-
- @Override
- public <T extends RESTResponse> T post(
- String path,
- RESTRequest body,
- Class<T> responseType,
- Map<String, String> headers,
- Consumer<ErrorResponse> errorHandler) {
- return execute(HTTPMethod.POST, path, null, body, errorHandler);
- }
-
- @Override
- public <T extends RESTResponse> T get(
- String path,
- Map<String, String> queryParams,
- Class<T> responseType,
- Map<String, String> headers,
- Consumer<ErrorResponse> errorHandler) {
- return execute(HTTPMethod.GET, path, queryParams, null, errorHandler);
- }
-
- @Override
- public void head(String path, Map<String, String> headers,
Consumer<ErrorResponse> errorHandler) {
- execute(HTTPMethod.HEAD, path, null, headers, errorHandler);
- }
-
- @Override
- public <T extends RESTResponse> T postForm(
- String path,
- Map<String, String> formData,
- Class<T> responseType,
- Map<String, String> headers,
- Consumer<ErrorResponse> errorHandler) {
- return execute(HTTPMethod.POST, path, null, formData, errorHandler);
+ response.setStatus(error.code());
+ RESTObjectMapper.mapper().writeValue(response.getWriter(), error);
+ return null;
}
@Override
@@ -627,11 +573,15 @@ public static <T extends RESTResponse> T
castResponse(Class<T> responseType, Obj
public static void configureResponseFromException(
Exception exc, ErrorResponse.Builder errorBuilder) {
- errorBuilder
- .responseCode(EXCEPTION_ERROR_CODES.getOrDefault(exc.getClass(), 500))
- .withType(exc.getClass().getSimpleName())
- .withMessage(exc.getMessage())
- .withStackTrace(exc);
+ var errorCode = EXCEPTION_ERROR_CODES.getOrDefault(exc.getClass(), 500);
+
errorBuilder.responseCode(errorCode).withType(exc.getClass().getSimpleName()).withMessage(exc.getMessage());
+ // avoid exposing stack traces for client errors but include them for
server errors to aid debugging
+ if (errorCode >= 500) {
+ LOG.error("A server error happened while processing REST request", exc);
+ errorBuilder.withStackTrace(exc);
+ } else {
+ LOG.info("A client error happened while processing REST request: {}",
exc.getMessage());
+ }
}
private static Namespace namespaceFromPathVars(Map<String, String> pathVars)
{
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
index e608ba5229a..21b155d65d8 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
@@ -20,10 +20,8 @@
package org.apache.iceberg.rest;
import java.io.IOException;
-import java.io.UncheckedIOException;
import java.util.Map;
import java.util.Optional;
-import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -77,29 +75,17 @@ protected void service(HttpServletRequest request,
HttpServletResponse response)
context.path(),
context.queryParams(),
context.body(),
- handle(response));
+ response);
if (responseBody != null) {
RESTObjectMapper.mapper().writeValue(response.getWriter(),
responseBody);
}
} catch (RuntimeException | IOException e) {
- // should be a RESTException but not able to see them through
dependencies
LOG.error("Error processing REST request", e);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
}
- private Consumer<ErrorResponse> handle(HttpServletResponse response) {
- return errorResponse -> {
- response.setStatus(errorResponse.code());
- try {
- RESTObjectMapper.mapper().writeValue(response.getWriter(),
errorResponse);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- };
- }
-
@Override
public void destroy() {
super.destroy();
