You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Andrew Purtell (JIRA)" <ji...@apache.org> on 2019/05/09 21:50:00 UTC
[jira] [Comment Edited] (HBASE-22274) Cell size limit check on
append should consider cell's previous size.
[ https://issues.apache.org/jira/browse/HBASE-22274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16836729#comment-16836729 ]
Andrew Purtell edited comment on HBASE-22274 at 5/9/19 9:49 PM:
----------------------------------------------------------------
v4 patch lgtm
One minor question, on this change:
{code}
+ int newCellSize = CellUtil.estimatedSerializedSizeOf(newCell);
+ if (newCellSize > this.maxCellSize) {
+ String msg = "Cell with size " + newCellSize + " exceeds limit of " +
+ this.maxCellSize + " bytes";
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(msg);
+ }
+ throw new DoNotRetryIOException(msg);
+ }
{code}
Should we be logging this at a higher log level, like INFO, or even WARN?
Whatever we decide here should be consistent with other warnings issued at the other sites where we do this size limit test. (I.e. if DEBUG there and we decide WARN, update to patch both warnings to WARN, or vice versa.)
was (Author: apurtell):
v4 patch lgtm
The test issue is clearer now.
One minor question, on this change:
{code}
+ int newCellSize = CellUtil.estimatedSerializedSizeOf(newCell);
+ if (newCellSize > this.maxCellSize) {
+ String msg = "Cell with size " + newCellSize + " exceeds limit of " +
+ this.maxCellSize + " bytes";
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(msg);
+ }
+ throw new DoNotRetryIOException(msg);
+ }
{code}
Should we be logging this at a higher log level, like INFO, or even WARN?
Whatever we decide here should be consistent with other warnings issued at the other sites where we do this size limit test. (I.e. if DEBUG there and we decide WARN, update to patch both warnings to WARN, or vice versa.)
> Cell size limit check on append should consider cell's previous size.
> ---------------------------------------------------------------------
>
> Key: HBASE-22274
> URL: https://issues.apache.org/jira/browse/HBASE-22274
> Project: HBase
> Issue Type: Bug
> Affects Versions: 3.0.0, 2.0.0, 1.3.5
> Reporter: Xu Cang
> Assignee: Xu Cang
> Priority: Minor
> Attachments: HBASE-22274-branch-1.001.patch, HBASE-22274-branch-1.002.patch, HBASE-22274-branch-1.003.patch, HBASE-22274-branch-1.003.patch, HBASE-22274-branch-1.004.patch, HBASE-22274-master.001.patch, HBASE-22274-master.002.patch, HBASE-22274-master.002.patch, HBASE-22274-master.003.patch
>
>
> Now we have cell size limit check based on this parameter *hbase.server.keyvalue.maxsize*
> One case was missing: appending to a cell only take append op's cell size into account against this limit check. we should check against the potential final cell size after the append.'
> It's easy to reproduce this :
>
> Apply this diff
>
> {code:java}
> diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java index 5a285ef6ba..8633177ebe 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java @@ -6455,7 +6455,7
> - t.append(new Append(ROW).addColumn(FAMILY, QUALIFIER, new byte[10 * 1024]));
> + t.append(new Append(ROW).addColumn(FAMILY, QUALIFIER, new byte[2 * 1024])); {code}
>
> Fix is to add this check in #reckonDeltas in HRegion class, where we have already got the appended cell's size.
> Will throw DoNotRetryIOException if checks is failed.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)