-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
MAPREDUCE-7368. DBOutputFormat.DBRecordWriter#write must throw except… #3671
Conversation
…ion when it fails When the exception is caught and printed (instead of propagated) the consumer has no way to tell write failed. This hides the failure and leads to problems which are difficult to debug and at the same time can cause data corruption.
💔 -1 overall
This message was automatically generated. |
} catch (SQLException e) { | ||
e.printStackTrace(); | ||
throw new RuntimeException("Failed to execute SQL statement for key " + key, e); |
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.
In other places. It is converting the SQLException to IOE.
catch (SQLException ex) { throw new IOException(ex.getMessage());
Can we keep the same behaviour, will that also solve the problem.
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.
Good point. I think it makes sense to keep things uniform so I changed it to IOException
in 1e67cec
💔 -1 overall
This message was automatically generated. |
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.
Thanx @zabetak for the update.
Changes LGTM.
Will commit by tomorrow, if no further comments
…ion when it fails. (apache#3671). Contributed by Stamatis Zampetakis. Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
Description of PR
When the exception is caught and printed (instead of propagated) the consumer has no way to tell write failed. This hides the failure and leads to problems which are difficult to debug and at the same time can cause data corruption.
More details in the JIRA MAPREDUCE-7368.
How was this patch tested?
Writing a well-contained test is not easy at the same time the change is straightforward so it may not be necessary.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?