-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 pending tests
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 :) |
@abstractdog, should we merge this or wait for HIVE-28135? |
LGTM, we can merge until I finalize HIVE-28135 |
|
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