HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

jbewing · 2023-08-09T03:12:59Z

What

This PR updates BufferedEncodedSeeker#seekToKeyInBlock (used extensively in the reverse scan path) to avoid calling CellUtil.cloneRow for off-heap Cell seek targets.

Implementation Notes

For private "findCommonPrefix*" methods, I used an invariant that the "right" cell is always a keyOnlyKv and therefore on-heap.

For the public "compareCommonPrefix*" methods, I added extra paths to them to account for either the left or right cell to be on or off-heap to keep the behavior optimized for any external callers. In practice, I believe these methods are internal to BufferedEncodedSeeker except for a few unit tests so I'd be open to reducing the branching to just optimize for the left cell being able to be on/off-heap. I'd love some feedback on this approach as it adds a fair amount of extra code that isn't strictly necessary for this PR.

I also added a test that is a clone of a current seekToKeyInBlock over a sample of data with the only variation being that the seek target cell is an off-heap cell instead of an on-heap one to give coverage over the new path here. Let me know if you'd like to see any more tests.

Testing

Some (naive and non-JMH) testing has been done of the new code path here. It looks to be in the range of about 20-30% faster than the old path for off-heap cell seek targets. Obviously, take these numbers with a grain of salt as they're not from JMH.

HBASE-28012

Apache-HBase · 2023-08-09T03:50:25Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 25s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+0 🆗	mvndep	0m 14s	Maven dependency ordering for branch
+1 💚	mvninstall	2m 50s	master passed
+1 💚	compile	2m 56s	master passed
+1 💚	checkstyle	0m 46s	master passed
+1 💚	spotless	0m 42s	branch has no errors when running spotless:check.
+1 💚	spotbugs	1m 52s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 33s	the patch passed
+1 💚	compile	2m 55s	the patch passed
+1 💚	javac	2m 55s	the patch passed
+1 💚	checkstyle	0m 45s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	9m 32s	Patch does not cause any errors with Hadoop 3.2.4 3.3.5.
+1 💚	spotless	0m 39s	patch has no errors when running spotless:check.
+1 💚	spotbugs	2m 12s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 16s	The patch does not generate ASF License warnings.
		34m 48s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux cf8bb8302e44 5.4.0-1101-aws #109~18.04.1-Ubuntu SMP Mon Apr 24 20:40:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `7cfa47d`
Default Java	Eclipse Adoptium-11.0.17+8
Max. process+thread count	80 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-08-09T07:19:39Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 25s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 13s	Maven dependency ordering for branch
+1 💚	mvninstall	2m 33s	master passed
+1 💚	compile	0m 53s	master passed
+1 💚	shadedjars	5m 4s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 36s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 20s	the patch passed
+1 💚	compile	0m 52s	the patch passed
+1 💚	javac	0m 52s	the patch passed
+1 💚	shadedjars	5m 0s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 34s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 45s	hbase-common in the patch passed.
+1 💚	unit	219m 13s	hbase-server in the patch passed.
		244m 12s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 3848d38a8d87 5.4.0-1101-aws #109~18.04.1-Ubuntu SMP Mon Apr 24 20:40:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `7cfa47d`
Default Java	Temurin-1.8.0_352-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/testReport/
Max. process+thread count	4613 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-08-09T07:23:50Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 28s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for branch
+1 💚	mvninstall	2m 43s	master passed
+1 💚	compile	1m 5s	master passed
+1 💚	shadedjars	4m 38s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 43s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 14s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 34s	the patch passed
+1 💚	compile	1m 5s	the patch passed
+1 💚	javac	1m 5s	the patch passed
+1 💚	shadedjars	4m 35s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 42s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 9s	hbase-common in the patch passed.
+1 💚	unit	222m 39s	hbase-server in the patch passed.
		248m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 6a821be5ea50 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `7cfa47d`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/testReport/
Max. process+thread count	4386 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/1/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java

hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java

Apache9 · 2023-08-13T09:50:40Z

Thanks for opening this PR.

Overall LGTM. Just a simple question and a follow on task.

Apache-HBase · 2023-08-14T22:04:22Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 10s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for branch
+1 💚	mvninstall	2m 45s	master passed
+1 💚	compile	2m 56s	master passed
+1 💚	checkstyle	0m 52s	master passed
+1 💚	spotless	0m 43s	branch has no errors when running spotless:check.
+1 💚	spotbugs	1m 58s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 29s	the patch passed
+1 💚	compile	2m 57s	the patch passed
+1 💚	javac	2m 57s	the patch passed
+1 💚	checkstyle	0m 49s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	8m 58s	Patch does not cause any errors with Hadoop 3.2.4 3.3.5.
+1 💚	spotless	0m 41s	patch has no errors when running spotless:check.
+1 💚	spotbugs	2m 14s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 19s	The patch does not generate ASF License warnings.
		34m 0s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux 3b10fdde423f 5.4.0-156-generic #173-Ubuntu SMP Tue Jul 11 07:25:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `20c9e4b`
Default Java	Eclipse Adoptium-11.0.17+8
Max. process+thread count	80 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-08-15T01:30:25Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 24s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 30s	Maven dependency ordering for branch
+1 💚	mvninstall	2m 25s	master passed
+1 💚	compile	0m 49s	master passed
+1 💚	shadedjars	4m 53s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 33s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 14s	the patch passed
+1 💚	compile	0m 49s	the patch passed
+1 💚	javac	0m 49s	the patch passed
+1 💚	shadedjars	4m 51s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 32s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 43s	hbase-common in the patch passed.
+1 💚	unit	215m 48s	hbase-server in the patch passed.
		239m 57s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux a0145901c91f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `20c9e4b`
Default Java	Temurin-1.8.0_352-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/testReport/
Max. process+thread count	4976 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-08-15T02:18:04Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 43s	Docker mode activated.
-0 ⚠️	yetus	0m 2s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 12s	Maven dependency ordering for branch
+1 💚	mvninstall	3m 30s	master passed
+1 💚	compile	1m 19s	master passed
+1 💚	shadedjars	5m 28s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 48s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for patch
+1 💚	mvninstall	3m 15s	the patch passed
+1 💚	compile	1m 16s	the patch passed
+1 💚	javac	1m 16s	the patch passed
+1 💚	shadedjars	5m 43s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 48s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 35s	hbase-common in the patch passed.
+1 💚	unit	256m 33s	hbase-server in the patch passed.
		287m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5347
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux e3be821c9669 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `20c9e4b`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/testReport/
Max. process+thread count	4410 (vs. ulimit of 30000)
modules	C: hbase-common hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5347/2/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

…dedSeeker (apache#5347) Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

…5347) Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1) (cherry picked from commit 9aa8b9b) Change-Id: I246d035e9138fa4c0927b6a0ecbaf1bc9f9a31ad

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker

b7df733

Apache9 reviewed Aug 13, 2023

View reviewed changes

hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java Show resolved Hide resolved

hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java Show resolved Hide resolved

Enforce that right Cell is always on-heap

458e99f

jbewing requested a review from Apache9 August 14, 2023 21:34

Apache9 approved these changes Aug 15, 2023

View reviewed changes

Apache9 merged commit 2fb2ae1 into apache:master Aug 15, 2023

Apache9 pushed a commit that referenced this pull request Aug 15, 2023

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker (#5347)

9aa8b9b

Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

Apache9 pushed a commit that referenced this pull request Aug 15, 2023

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker (#5347)

e61207a

Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

Apache9 pushed a commit that referenced this pull request Aug 15, 2023

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker (#5347)

bf945f2

Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

Apache9 pushed a commit that referenced this pull request Aug 15, 2023

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker (#5347)

a5aab42

Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

bbeaudreault pushed a commit to HubSpot/hbase that referenced this pull request Aug 21, 2023

HubSpot Backport: HBASE-28012 Avoid CellUtil.cloneRow in BufferedEnco…

328e339

…dedSeeker (apache#5347) Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 2fb2ae1)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

Uh oh!

jbewing commented Aug 9, 2023 •

edited

Loading

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Uh oh!

Uh oh!

Apache9 commented Aug 13, 2023

Uh oh!

Apache-HBase commented Aug 14, 2023

Uh oh!

Apache-HBase commented Aug 15, 2023

Uh oh!

Apache-HBase commented Aug 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

HBASE-28012 Avoid CellUtil.cloneRow in BufferedEncodedSeeker #5347

Uh oh!

Conversation

jbewing commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Implementation Notes

Testing

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Apache-HBase commented Aug 9, 2023

Uh oh!

Uh oh!

Uh oh!

Apache9 commented Aug 13, 2023

Uh oh!

Apache-HBase commented Aug 14, 2023

Uh oh!

Apache-HBase commented Aug 15, 2023

Uh oh!

Apache-HBase commented Aug 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jbewing commented Aug 9, 2023 •

edited

Loading