Skip to content

HIVE-29563: Iceberg REST Catalog's error logs are too verbose#6425

Merged
okumin merged 5 commits intoapache:masterfrom
okumin:HIVE-29563-rest-logging
May 6, 2026
Merged

HIVE-29563: Iceberg REST Catalog's error logs are too verbose#6425
okumin merged 5 commits intoapache:masterfrom
okumin:HIVE-29563-rest-logging

Conversation

@okumin
Copy link
Copy Markdown
Contributor

@okumin okumin commented Apr 12, 2026 •
edited
Loading

What changes were proposed in this pull request?

Stop logging every client-side error, i.e., 40x errors.

https://issues.apache.org/jira/browse/HIVE-29563

Why are the changes needed?

As explained in HIVE-29563, the current REST API prints the entire stack trace on every error.

apache/iceberg has the same problem and sent a patch, but it has not been reviewed: apache/iceberg#14908

Does this PR introduce any user-facing change?

It includes a change for HMS administrators.

How was this patch tested?

standalone-metastore/packaging/src/docker/build.sh -repo local
docker run --rm --name hive-standalone-metastore \
  -p 9083:9083 \
  -p 9001:9001 \
  local/hive:standalone-metastore-4.3.0-SNAPSHOT
curl -i http://localhost:9001/iceberg/v1/namespaces/default/tables/does_not_exist

@okumin okumin force-pushed the HIVE-29563-rest-logging branch from 9debb2e to 1daa058 Compare April 14, 2026 00:14
okumin
okumin commented Apr 14, 2026
View reviewed changes
Comment thread ...etastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
// The caller is responsible for closing the underlying catalog backing this REST catalog.
response.setStatus(error.code());
RESTObjectMapper.mapper().writeValue(response.getWriter(), error);
return null;
Copy link
Copy Markdown
Contributor Author

@okumin okumin Apr 14, 2026

Choose a reason for hiding this comment

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

RESTServerCatalogAdapter on apache/iceberg extends a RESTClient. I don't know the exact reason. The trick is used in some integration tests.
Anyway, as our adapter never behaves as a client, I removed the dependency on RESTClient. As a good side effect, this change ensures only a single path invokes RESTServerCatalogAdapter#execute and allows us to simplify the implementation.

@okumin okumin changed the title [WIP] HIVE-29563: Iceberg REST Catalog's error logs are too verbose HIVE-29563: Iceberg REST Catalog's error logs are too verbose Apr 15, 2026
@okumin okumin marked this pull request as ready for review April 15, 2026 00:31
@ayushtkn
Copy link
Copy Markdown
Member

ayushtkn commented Apr 28, 2026

@henrib can you check once

henrib
henrib reviewed Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@henrib henrib left a comment

Choose a reason for hiding this comment

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

If this does not require any test changes, LGTM 👍

Comment thread ...etastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
okumin
okumin commented Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@okumin okumin left a comment

Choose a reason for hiding this comment

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

Thanks for your review! It will make upcoming changes easy to review

Comment thread ...etastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
@okumin okumin mentioned this pull request May 2, 2026
Draft
ayushtkn
ayushtkn approved these changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

@okumin
Copy link
Copy Markdown
Contributor Author

okumin commented May 6, 2026

I merged the master branch and resolved conflicts.

Additionally, I found that we can't see anything when a 5xx error occurs. I updated it so we can see a similar log on server errors, and INFO log for non-server errors for convenience.

2026-05-06T04:41:44,152  INFO [qtp1381730594-59] rest.HMSCatalogAdapter: A client error happened while processing REST request: Table does not exist: default.does_not_exist

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@okumin okumin merged commit cf1d6d8 into apache:master May 6, 2026
6 checks passed
@okumin okumin deleted the HIVE-29563-rest-logging branch May 6, 2026 12:44
@okumin
Copy link
Copy Markdown
Contributor Author

okumin commented May 6, 2026

Merged. @ayushtkn @henrib Thanks for your reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ayushtkn ayushtkn ayushtkn approved these changes

+1 more reviewer

@henrib henrib henrib left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tests passed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@okumin @ayushtkn @henrib @asf-ci-hive