-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-29097 Add error logging when put creation fails #6638
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
928dd0e
to
0cc31dd
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@virajjasani FYI |
try { | ||
Put put = new Put(metaKeyForRegion, ts); | ||
return addRegionInfo(put, regionInfo); | ||
} catch (IllegalArgumentException ex) { |
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.
why logging only for IllegalArgumentException ? why not IOException ?
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.
We want to catch IllegalArgumentException because that is what our code throws
public Put(byte[] rowArray, int rowOffset, int rowLength, long ts) {
checkRow(rowArray, rowOffset, rowLength);
this.row = Bytes.copy(rowArray, rowOffset, rowLength);
this.ts = ts;
if (ts < 0) {
throw new IllegalArgumentException("Timestamp cannot be negative. ts=" + ts);
}
}
static byte[] checkRow(final byte[] row, final int offset, final int length) {
if (row == null) {
throw new IllegalArgumentException("Row buffer is null");
}
if (length == 0) {
throw new IllegalArgumentException("Row length is 0");
}
if (length > HConstants.MAX_ROW_LENGTH) {
throw new IllegalArgumentException(
"Row length " + length + " is > " + HConstants.MAX_ROW_LENGTH);
}
return row;
}
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.
Ohh it is IllegalArgumentException not IllegalArgumentIOException. It is an unchecked exception. I wonder why they used unchecked one. And there are not many places where we handled IllegalArgumentException.
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
No description provided.