Skip to content

HBASE-26527 ArrayIndexOutOfBoundsException in KeyValueUtil.copyToNewK… - #3904

Merged
Apache9 merged 2 commits into
apache:masterfrom
stoty:HBASE-26527
Dec 6, 2021
Merged

HBASE-26527 ArrayIndexOutOfBoundsException in KeyValueUtil.copyToNewK…#3904
Apache9 merged 2 commits into
apache:masterfrom
stoty:HBASE-26527

Conversation

@stoty

@stotystoty commented Dec 1, 2021

Copy link
Copy Markdown
Contributor

…eyValue()

Change-Id: I33099354f40b04b0f4f79f5cbd32445fe9d0d818

…eyValue()
Change-Id: I33099354f40b04b0f4f79f5cbd32445fe9d0d818
@stoty
stoty marked this pull request as draft December 1, 2021 12:55
@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 26sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall4m 53smaster passed
+1 💚compile0m 28smaster passed
+1 💚shadedjars8m 56sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 27smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 40sthe patch passed
+1 💚compile0m 28sthe patch passed
+1 💚javac0m 28sthe patch passed
+1 💚shadedjars8m 16spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 25sthe patch passed
_ Other Tests _
+1 💚unit2m 7shbase-common in the patch passed.
32m 9s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#3904
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 32db26357eef 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / d1762f5
Default JavaAdoptOpenJDK-11.0.10+9
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/testReport/
Max. process+thread count300 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 35sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall4m 42smaster passed
+1 💚compile0m 28smaster passed
+1 💚shadedjars9m 18sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 25smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 30sthe patch passed
+1 💚compile0m 26sthe patch passed
+1 💚javac0m 26sthe patch passed
+1 💚shadedjars9m 44spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 24sthe patch passed
_ Other Tests _
+1 💚unit2m 0shbase-common in the patch passed.
33m 48s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#3904
Optional Testsjavac javadoc unit shadedjars compile
unameLinux a76b2da62ea1 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / d1762f5
Default JavaAdoptOpenJDK-1.8.0_282-b08
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/testReport/
Max. process+thread count360 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 28sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall4m 10smaster passed
+1 💚compile0m 52smaster passed
+1 💚checkstyle0m 27smaster passed
+1 💚spotbugs0m 48smaster passed
_ Patch Compile Tests _
+1 💚mvninstall3m 47sthe patch passed
+1 💚compile0m 48sthe patch passed
+1 💚javac0m 48sthe patch passed
+1 💚checkstyle0m 26sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck19m 23sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs0m 56sthe patch passed
_ Other Tests _
+1 💚asflicense0m 16sThe patch does not generate ASF License warnings.
40m 28s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#3904
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
unameLinux 5186b05cc77b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / d1762f5
Default JavaAdoptOpenJDK-1.8.0_282-b08
Max. process+thread count96 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/1/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty
stoty marked this pull request as ready for review December 2, 2021 10:21
@Apache9

Copy link
Copy Markdown
Contributor

Checked the code in KeyValueUtils, there is a method getSerializedSize

 public static int getSerializedSize(Cell cell, boolean withTags) {
if (withTags) {
return cell.getSerializedSize();
}
if (cell instanceof ExtendedCell) {
return ((ExtendedCell) cell).getSerializedSize(withTags);
}
return length(cell.getRowLength(), cell.getFamilyLength(), cell.getQualifierLength(),
cell.getValueLength(), cell.getTagsLength(), withTags);
}

It seems that if withTags is true, we will always call cell.getSerializedSize directly. I guess this is why we use getSerializedSize there.

So let's add some comments to say that, the byte array is not used for serializaing, as it will copy all the fields such as column family, qualifier, etc, so it is not suitable to call getSerializedSize here, we should calculate it with all the fields.

Thanks.

@stoty

stoty commented Dec 6, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @Apache9 , I have added the comment.

@Apache9

Copy link
Copy Markdown
Contributor

Thanks for the review @Apache9 , I have added the comment.

Forgot to push it? Seems the PR remains unchanged...

Change-Id: Ibf9419d391560a209404bb7dcc6b583502d12bda
@stoty

stoty commented Dec 6, 2021

Copy link
Copy Markdown
ContributorAuthor

@Apache9 Yes, I pushed to the wrong repo.
Please check now.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 31sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall4m 56smaster passed
+1 💚compile0m 26smaster passed
+1 💚shadedjars9m 13sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 28smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 28sthe patch passed
+1 💚compile0m 27sthe patch passed
+1 💚javac0m 27sthe patch passed
+1 💚shadedjars8m 21spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 26sthe patch passed
_ Other Tests _
+1 💚unit2m 5shbase-common in the patch passed.
32m 24s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#3904
Optional Testsjavac javadoc unit shadedjars compile
unameLinux d8c31e4bdb1b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dcd622d
Default JavaAdoptOpenJDK-11.0.10+9
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/testReport/
Max. process+thread count297 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 1sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall4m 31smaster passed
+1 💚compile0m 23smaster passed
+1 💚shadedjars9m 3sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 22smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 16sthe patch passed
+1 💚compile0m 25sthe patch passed
+1 💚javac0m 25sthe patch passed
+1 💚shadedjars9m 9spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 21sthe patch passed
_ Other Tests _
+1 💚unit2m 1shbase-common in the patch passed.
32m 33s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#3904
Optional Testsjavac javadoc unit shadedjars compile
unameLinux fbe740a80a17 4.15.0-147-generic #151-Ubuntu SMP Fri Jun 18 19:21:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dcd622d
Default JavaAdoptOpenJDK-1.8.0_282-b08
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/testReport/
Max. process+thread count228 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec8m 57sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 1sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall5m 36smaster passed
+1 💚compile0m 55smaster passed
+1 💚checkstyle0m 31smaster passed
+1 💚spotbugs0m 56smaster passed
_ Patch Compile Tests _
+1 💚mvninstall5m 7sthe patch passed
+1 💚compile0m 51sthe patch passed
+1 💚javac0m 51sthe patch passed
+1 💚checkstyle0m 29sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck26m 35sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs1m 1sthe patch passed
_ Other Tests _
+1 💚asflicense0m 14sThe patch does not generate ASF License warnings.
61m 21s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#3904
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
unameLinux 587f6a4aefbe 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dcd622d
Default JavaAdoptOpenJDK-1.8.0_282-b08
Max. process+thread count86 (vs. ulimit of 30000)
modulesC: hbase-common U: hbase-common
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3904/2/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Apache9 merged commit d9315fa into apache:masterDec 6, 2021
Apache9 pushed a commit that referenced this pull request Dec 6, 2021
…eyValue() (#3904)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
Apache9 pushed a commit that referenced this pull request Dec 6, 2021
…eyValue() (#3904)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
guangxuCheng pushed a commit to guangxuCheng/hbase that referenced this pull request Mar 28, 2022
vinayakphegde pushed a commit to vinayakphegde/hbase that referenced this pull request Apr 4, 2024
…eyValue() (apache#3904)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@stoty@Apache-HBase@Apache9@apurtell