HBASE-28463 Time Based Priority for BucketCache by wchevreuil · Pull Request #7192 · apache/hbase

wchevreuil · 2025-08-05T19:38:59Z

No description provided.

…t-cache (#5793) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

…or Time Range Data Tiering (#5809) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

…5829) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

…on paths (#5866) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewed-by: Janardhan Hugund <janardhan.hungund@cloudera.com>

…ode paths. (#5905) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

taklwu

few questions below and inlines, otherwise LGTM

according from #5829, there is a file TestPrefetch.java in the diff, where did we remove it from this PR?

taklwu · 2025-08-13T16:50:36Z

 BlockType.BlockCategory category = hfileBlock.getBlockType().getCategory();
 final boolean cacheCompressed = cacheConf.shouldCacheCompressed(category);
- final boolean cacheOnRead = cacheConf.shouldCacheBlockOnRead(category);
+ final boolean cacheOnRead =


this part is different from #5905 , where cacheOnRead has been defined but it does not use in line#1391 and line#1405, is it expected?
in other words, is these two APIs of cacheConf.shouldCacheBlockOnRead(category) and cacheConf.shouldCacheBlockOnRead(category, getHFileInfo(), conf) the same?

this part is different from #5905 , where cacheOnRead has been defined but it does not use in line#1391 and line#1405, is it expected?
Sorry, ain't following you. This seems same code we added in PR 5905, line #1351 of HFileReaderImpl.

in other words, is these two APIs of cacheConf.shouldCacheBlockOnRead(category) and cacheConf.shouldCacheBlockOnRead(category, getHFileInfo(), conf) the same?
Those are not the same. The former just checks for the block type, whilst the latter performs additional checks to figure if the data block is already cached, in order to avoid redundant caching from file system.

yeah, sorry I didn't explain my comment with links,
about the cacheOnRead , in the old PR #5905 , cacheOnRead has been used within the if block if (cacheOnly && cacheCompressed && cacheOnRead) {https://github.com/apache/hbase/pull/5905/files#diff-c910a49e7d962e49b199d22f38e1cd78d867351208b86bffdd56ffb9aa1aa596R1358 , but in this PR, we don't see those usage of cacheOnRead, did I miss anything?

Ok, I think I got it now. This is probably due to the rebase on top of master branch. Master branch had this HBASE-28596, which was missing on the feature branch when PR #5905 was merged. So when resolving this conflict in the rebase, I simply picked the master version.
Reading this now, seems wrong to me, we should do as PR #595 was doing. I'm going to update it.

thanks for fixing it. LGTM now

taklwu · 2025-08-13T16:57:41Z

+ // Since HBASE-28466, we call fileInfo.initMetaAndIndex inside HFilePreadReader,
+ // which reads some blocks and increment the counters, so we need to reset it here.
+ ThreadLocalServerSideScanMetrics.getBytesReadFromFsAndReset();
+ ThreadLocalServerSideScanMetrics.getBlockReadOpsCountAndReset();


it's not part of #7124 , can you point out where does this come from ? new changes to fix tests when HBASE-28466 is included?

Yeah, these are changes introduced in this PR last commit to fix the tests for HBASE-28466 changes.

wchevreuil · 2025-08-26T09:37:07Z

few questions below and inlines, otherwise LGTM
according from HBASE-28468: Integrate the data-tiering logic into cache evictions. #5829, there is a file TestPrefetch.java in the diff, where did we remove it from this PR?

That change from #5829 was removed when resolving conflicts from HBASE-28804. The code introduced on HBASE-28804 makes change from #5829 irrelevant.

…ure branch (#7124) This is the whole custom tiering implementation and involves the following individual works: * HBASE-29412 Extend date tiered compaction to allow for tiering by values other than cell timestamp * HBASE-29413 Implement a custom qualifier tiered compaction * HBASE-29414 Refactor DataTieringManager to make priority logic pluggable * HBASE-29422 Implement selectMinorCompation in CustomCellDateTieredCompactionPolicy * HBASE-29424 Implement configuration validation for custom tiering compactions * HBASE-29425 Refine and polish code * HBASE-29426 Define a tiering value provider and refactor custom tiered compaction related classes * HBASE-28463 Rebase time based priority branch (HBASE-28463) with latest master (and fix conflicts) Co-authored-by: Janardhan Hungund <janardhan.hungund@cloudera.com> Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>

Apache-HBase · 2025-08-26T20:42:14Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 32s		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 38s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 36s		master passed
+1 💚	compile	4m 6s		master passed
+1 💚	checkstyle	0m 55s		master passed
+1 💚	spotbugs	2m 19s		master passed
+1 💚	spotless	0m 52s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 5s		the patch passed
+1 💚	compile	4m 3s		the patch passed
+1 💚	javac	4m 3s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 40s	/results-checkstyle-hbase-server.txt	hbase-server: The patch generated 16 new + 30 unchanged - 0 fixed = 46 total (was 30)
+1 💚	spotbugs	2m 30s		the patch passed
+1 💚	hadoopcheck	11m 48s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 48s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 20s		The patch does not generate ASF License warnings.
		44m 30s

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

This message was automatically generated.

Apache-HBase · 2025-08-27T00:23:31Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 29s		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 10s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 19s		master passed
+1 💚	compile	1m 17s		master passed
+1 💚	javadoc	0m 45s		master passed
+1 💚	shadedjars	6m 9s		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 7s		the patch passed
+1 💚	compile	1m 17s		the patch passed
+1 💚	javac	1m 17s		the patch passed
+1 💚	javadoc	0m 42s		the patch passed
+1 💚	shadedjars	6m 6s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	2m 15s		hbase-common in the patch passed.
+1 💚	unit	234m 39s		hbase-server in the patch passed.
		265m 51s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7192/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7192
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 6892ebfbdca4 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 / `6fdffaf`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7192/4/testReport/
Max. process+thread count	4450 (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-7192/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.

wchevreuil · 2025-08-27T19:13:45Z

Any further comments, @taklwu ?

vinayakphegdeand others added 7 commits August 5, 2025 15:19

HBASE-28465Implementation of framework for time-based priority bucke…
915a507
…t-cache (#5793) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

HBASE-28505Implement enforcement to require Date Tiered Compaction f…
7525682
…or Time Range Data Tiering (#5809) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

HBASE-28466Integration of time-based priority logic of bucket cache …
1289101
…in prefetch functionality of HBase (#5808) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

HBASE-28468: Integrate the data-tiering logic into cache evictions. (#…
fb0c315
…5829) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

HBASE-28535: Add a region-server wide key to enable data-tiering. (#5856
6ea6144
) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

HBASE-28469: Integration of time-based priority caching into compacti…
21ff195
…on paths (#5866) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewed-by: Janardhan Hugund <janardhan.hungund@cloudera.com>

HBASE-28467: Add time-based priority caching checks for cacheOnRead c…
dc71f85
…ode paths. (#5905) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

This comment has been minimized.
Sign in to view

wchevreuil force-pushed the HBASE-28463 branch from 98a0296 to 3f75884Compare August 7, 2025 16:34

This comment has been minimized.
Sign in to view

wchevreuil force-pushed the HBASE-28463 branch from 622e52d to 7b7728dCompare August 7, 2025 19:24

This comment has been minimized.
Sign in to view

taklwu reviewed Aug 13, 2025
View reviewed changes

wchevreuil force-pushed the HBASE-28463 branch from 7b7728d to 6fdffafCompare August 26, 2025 19:48

wchevreuil requested a review from taklwu August 28, 2025 19:23

taklwu approved these changes Aug 28, 2025
View reviewed changes

wchevreuil merged commit fb5f919 into masterSep 1, 2025
1 check passed

Uh oh!

Conversation

wchevreuil commented Aug 5, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

taklwu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wchevreuil commented Aug 26, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Apache-HBase commented Aug 26, 2025

Uh oh!

Apache-HBase commented Aug 27, 2025

Uh oh!

wchevreuil commented Aug 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

taklwu left a comment •
edited
Loading

wchevreuil commented Aug 26, 2025•
edited
Loading