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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −2
Open

HBASE-29097 Add error logging when put creation fails #6638

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,8 +636,16 @@ public static Put makePutFromRegionInfo(RegionInfo regionInfo) throws IOExceptio
* @throws IllegalArgumentException when the provided RegionInfo is not the default replica.
*/
public static Put makePutFromRegionInfo(RegionInfo regionInfo, long ts) throws IOException {
return addRegionInfo(new Put(CatalogFamilyFormat.getMetaKeyForRegion(regionInfo), ts),
regionInfo);
byte[] metaKeyForRegion = CatalogFamilyFormat.getMetaKeyForRegion(regionInfo);
try {
Put put = new Put(metaKeyForRegion, ts);
return addRegionInfo(put, regionInfo);
} catch (IllegalArgumentException ex) {
Copy link
Contributor

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 ?

Copy link
Contributor Author

@mnpoonia mnpoonia Jan 28, 2025 •
edited
Loading

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;
  }

Copy link
Contributor

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.

LOG.error(
"Got exception while creating put for regioninfo {}." + "meta key for regioninfo is {}",
regionInfo.getRegionNameAsString(), metaKeyForRegion);
throw ex;
}
}

/**
Expand Down