HBASE-28600 Introduce hfile.block.cache.memory.size configuration by jinhyukify · Pull Request #6422 · apache/hbase

jinhyukify · 2024-10-30T15:07:45Z

Introduce a new configuration, hfile.block.cache.memory.size, which allows setting the block cache size with an exact byte value or a human-readable storage size.

ndimiduk

This looks really good. I have just some small comments. Have you tried it out? Is it working for you?

ndimiduk · 2024-12-06T09:53:45Z

- if (usage != null) {
- max = usage.getMax();
- }
+ final long heapMax = usage != null ? usage.getMax() : -1;


Should we throw when heapMax == -1? We don't have any safety on subsequent divide by -1, below.

What do you think of immediately returning -1 in this case?
We expect this method to return a negative value when the cache is disabled.
hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheFactory.java
Lines 117 to 120 in 1880cb3
finallongcacheSize = MemorySizeUtil.getOnHeapCacheSize(c);
if (cacheSize < 0) {
returnnull;
}

ndimiduk · 2024-12-06T09:55:20Z

 if (blockCachePercent < blockCachePercentMinRange) {
 LOG.warn("Setting " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " to " + blockCachePercent
- + ", same value as " + HFILE_BLOCK_CACHE_SIZE_KEY
+ + " (lookup order: " + HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + " -> "


nit: please cleanup any modified log lines to use modern style parameter substitution.

Thank you. updated.

jinhyukify

Have you tried it out? Is it working for you?

I briefly wrote down the developer test I performed on my local machine.
You can check it in this comment

jinhyukify · 2024-12-08T08:01:49Z

 if (blockCachePercent < blockCachePercentMinRange) {
 LOG.warn("Setting " + BLOCK_CACHE_SIZE_MIN_RANGE_KEY + " to " + blockCachePercent
- + ", same value as " + HFILE_BLOCK_CACHE_SIZE_KEY
+ + " (lookup order: " + HFILE_BLOCK_CACHE_MEMORY_SIZE_KEY + " -> "


Thank you. updated.

ndimiduk

Nice thank you!

Apache-HBase · 2024-12-10T11:01:01Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 31s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ master Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 12s		master passed
+1 💚	compile	3m 46s		master passed
+1 💚	checkstyle	0m 49s		master passed
+1 💚	spotbugs	2m 2s		master passed
+0 🆗	refguide	2m 20s		branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚	spotless	0m 46s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 3s		the patch passed
+1 💚	compile	3m 46s		the patch passed
+1 💚	javac	3m 46s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 48s		the patch passed
+1 💚	xmllint	0m 0s		No new issues.
+1 💚	spotbugs	2m 21s		the patch passed
+0 🆗	refguide	2m 4s		patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚	hadoopcheck	11m 28s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 44s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 17s		The patch does not generate ASF License warnings.
		45m 39s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6422/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#6422
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint refguide
uname	Linux c194f512c438 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 / `a401560`
Default Java	Eclipse Adoptium-17.0.11+9
refguide	https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6422/4/yetus-general-check/output/branch-site/book.html
refguide	https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6422/4/yetus-general-check/output/patch-site/book.html
Max. process+thread count	86 (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-6422/4/console
versions	git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2024-12-10T11:07:43Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 43s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ master Compile Tests _
+0 🆗	mvndep	0m 15s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 43s		master passed
+1 💚	compile	1m 17s		master passed
+1 💚	javadoc	0m 45s		master passed
+1 💚	shadedjars	5m 0s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 8s		the patch passed
+1 💚	compile	1m 17s		the patch passed
+1 💚	javac	1m 17s		the patch passed
+1 💚	javadoc	0m 44s		the patch passed
+1 💚	shadedjars	4m 51s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	2m 6s		hbase-common in the patch passed.
-1 ❌	unit	26m 42s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
		52m 17s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6422/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#6422
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 09253916f505 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `a401560`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6422/4/testReport/
Max. process+thread count	2036 (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-6422/4/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

ndimiduk · 2024-12-10T14:24:58Z

I think that this change is benign enough for our active release lines. @apurtell do you have any objection to adding this to branch-2.5?

…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…6529) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: JinHyuk Kim <tk5641@naver.com>

…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

#6422)" to branch-2 (#6544) Signed-off-by: Andrew Purtell <apurtell@apache.org>

#6422)" to branch-2.6 (#6545) Signed-off-by: Andrew Purtell <apurtell@apache.org>

#6422)" to branch-2.5 (#6592) Signed-off-by: Andrew Purtell <apurtell@apache.org>

apache#6422)" to branch-2 (apache#6544) Signed-off-by: Andrew Purtell <apurtell@apache.org>

apache#6422)" to branch-2.5 (apache#6592) Signed-off-by: Andrew Purtell <apurtell@apache.org>

HBASE-28600 Introduce hfile.block.cache.memory.size configuration
05c8c8a

This comment has been minimized.
Sign in to view

ndimiduk requested changes Dec 6, 2024
View reviewed changes

jinhyukify commented Dec 8, 2024
View reviewed changes

This comment has been minimized.
Sign in to view

jinhyukify added 3 commits December 8, 2024 22:19

Updated log messages to use parameter replacement
a71324b

Enhance exception messages for improved clarity
bfa1b1f

Return -1 if MemoryUsage is null
a401560

jinhyukify force-pushed the HBASE-28600 branch from d5d85df to a401560Compare December 8, 2024 13:22

This comment has been minimized.
Sign in to view

jinhyukify requested a review from ndimiduk December 9, 2024 05:31

ndimiduk approved these changes Dec 9, 2024
View reviewed changes

ndimiduk merged commit c4fed77 into apache:masterDec 10, 2024

ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Dec 10, 2024
HBASE-28600 Enable setting blockcache on-heap sizes in bytes (apache#…
94990d6
…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Dec 10, 2024
HBASE-28600 Enable setting blockcache on-heap sizes in bytes (apache#…
425fc4c
…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Dec 10, 2024
HBASE-28600 Enable setting blockcache on-heap sizes in bytes (apache#…
b3b3f90
…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

ndimiduk added a commit that referenced this pull request Dec 13, 2024
HBASE-28600Enable setting blockcache on-heap sizes in bytes (#6422) (#…
44b64e4
…6529) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: JinHyuk Kim <tk5641@naver.com>

gvprathyusha6 pushed a commit to gvprathyusha6/hbase that referenced this pull request Dec 19, 2024
HBASE-28600 Enable setting blockcache on-heap sizes in bytes (apache#…
ef650ab
…6422) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

apurtell pushed a commit that referenced this pull request Jan 14, 2025
Backport "HBASE-28600Enable setting blockcache on-heap sizes in bytes (
c816083
#6422)" to branch-2 (#6544) Signed-off-by: Andrew Purtell <apurtell@apache.org>

apurtell pushed a commit that referenced this pull request Jan 14, 2025
Backport"HBASE-28600Enable setting blockcache on-heap sizes in bytes (
41a9867
#6422)" to branch-2.6 (#6545) Signed-off-by: Andrew Purtell <apurtell@apache.org>

apurtell pushed a commit that referenced this pull request Jan 14, 2025
Backport"HBASE-28600Enable setting blockcache on-heap sizes in bytes (
ec96e70
#6422)" to branch-2.5 (#6592) Signed-off-by: Andrew Purtell <apurtell@apache.org>

mokai87 pushed a commit to mokai87/hbase that referenced this pull request Aug 7, 2025
Backport "HBASE-28600 Enable setting blockcache on-heap sizes in bytes (
bd1fc1f
apache#6422)" to branch-2 (apache#6544) Signed-off-by: Andrew Purtell <apurtell@apache.org>

sanjeet006py pushed a commit to sanjeet006py/hbase that referenced this pull request Sep 26, 2025
Backport" HBASE-28600 Enable setting blockcache on-heap sizes in bytes (
4bb33e6
apache#6422)" to branch-2.5 (apache#6592) Signed-off-by: Andrew Purtell <apurtell@apache.org>

	finallongcacheSize = MemorySizeUtil.getOnHeapCacheSize(c);
	if (cacheSize < 0) {
	returnnull;
	}

Uh oh!

Conversation

jinhyukify commented Oct 30, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

ndimidukDec 6, 2024

Choose a reason for hiding this comment

Uh oh!

jinhyukifyDec 9, 2024

Choose a reason for hiding this comment

Uh oh!

ndimidukDec 6, 2024

Choose a reason for hiding this comment

Uh oh!

jinhyukifyDec 8, 2024

Choose a reason for hiding this comment

Uh oh!

jinhyukify left a comment

Choose a reason for hiding this comment

Uh oh!

jinhyukifyDec 8, 2024

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Dec 10, 2024

Uh oh!

Apache-HBase commented Dec 10, 2024

Uh oh!

ndimiduk commented Dec 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants