Skip to content
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

HIVE-28133: Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderException #5139

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

deniskuzZ
Copy link
Member

@deniskuzZ deniskuzZ commented Mar 20, 2024

What changes were proposed in this pull request?

Logging improvements

Why are the changes needed?

Troubleshooting

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

on a cluster

@deniskuzZ deniskuzZ changed the title Log the original exception in handleRecordReaderNextException Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderNextException Mar 20, 2024
@deniskuzZ deniskuzZ changed the title Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderNextException HIVE-28133: Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderNextException Mar 20, 2024
@abstractdog abstractdog self-requested a review March 20, 2024 12:14
Copy link
Contributor

@abstractdog abstractdog left a comment

Choose a reason for hiding this comment

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

+1 pending tests

@abstractdog
Copy link
Contributor

LGTM, but created HIVE-28135 as a followup

@deniskuzZ
Copy link
Member Author

deniskuzZ commented Mar 20, 2024

LGTM, but created HIVE-28135 as a followup

it seems like a dead code with 0 implementations. should we then drop the whole stuff in 1 go? or maybe someone adds the handlers via aux jars?

@deniskuzZ deniskuzZ changed the title HIVE-28133: Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderNextException HIVE-28133: Log the original exception in HiveIOExceptionHandlerUtil#handleRecordReaderException Mar 20, 2024
@abstractdog
Copy link
Contributor

LGTM, but created HIVE-28135 as a followup

it seems like a dead code with 0 implementations. should we then drop the whole stuff in 1 go? or maybe someone adds the handlers via aux jars?

even if they add it via aux jars, I still don't get the point of defining flexible exception handlers for record reader exceptions, not even a single google match that made sense, so I believe we can let this awesome handler chain go :)

@deniskuzZ
Copy link
Member Author

@abstractdog, should we merge this or wait for HIVE-28135?

@abstractdog
Copy link
Contributor

LGTM, we can merge until I finalize HIVE-28135

Copy link

sonarcloud bot commented Apr 11, 2024

Quality Gate Passed Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@deniskuzZ deniskuzZ merged commit a3e5298 into apache:master Apr 15, 2024
5 checks passed
@deniskuzZ deniskuzZ deleted the logging branch April 15, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants