Skip to content

HBASE-28596 Optimise BucketCache usage upon regions splits/merges. (#5906)#6508

Merged
wchevreuil merged 1 commit intoapache:branch-2.6from
wchevreuil:HBASE-28596-branch-2
Jul 1, 2025
Merged

HBASE-28596 Optimise BucketCache usage upon regions splits/merges. (#5906)#6508
wchevreuil merged 1 commit intoapache:branch-2.6from
wchevreuil:HBASE-28596-branch-2

Conversation

@wchevreuil
Copy link
Copy Markdown
Contributor

@wchevreuil wchevreuil commented Dec 2, 2024

No description provided.

@Apache-HBase

This comment has been minimized.

Sign in to view
@bbeaudreault bbeaudreault requested a review from ndimiduk December 2, 2024 23:03
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
ragarkar
ragarkar approved these changes Jan 8, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ragarkar ragarkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
taklwu
taklwu previously approved these changes Jun 23, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@taklwu taklwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems the same as bd9053c#diff-c910a49e7d962e49b199d22f38e1cd78d867351208b86bffdd56ffb9aa1aa596 (the minor difference the extra spacing.)

hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java Outdated
// master hosted regions, like the master procedures store wouldn't have a block cache
// Prefetch file blocks upon open if requested
if (cacheConf.getBlockCache().isPresent() && cacheConf.shouldPrefetchOnOpen()) {

Copy link
Copy Markdown
Contributor

@taklwu taklwu Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we need this spacing ?

@taklwu taklwu self-requested a review June 23, 2025 23:41
@taklwu taklwu dismissed their stale review June 23, 2025 23:42

Hi Wellington, the backport should be fine but the patch cannot be build, can you check it ?

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
charlesconnell
charlesconnell approved these changes Jun 25, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@charlesconnell charlesconnell left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hubspot has been using this patch in production for some months. This successfully addresses the performance regression identified in HBASE-29012, and we haven't noticed any negative side effects.

@wchevreuil
Copy link
Copy Markdown
Contributor Author

wchevreuil commented Jun 26, 2025

Hubspot has been using this patch in production for some months. This successfully addresses the performance regression identified in HBASE-29012, and we haven't noticed any negative side effects.

Thanks for confirming @charlesconnell ! I will address the issues from the conflicts and once we get a green pre commit run, will get it merged.

@wchevreuil
HBASE-28596 Optimise BucketCache usage upon regions splits/merges. (a…
05d3993 
…pache#5906)

Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Reviewed-by: Duo Zhang <zhangduo@apache.org>
Change-Id: I6d6fc84c0e526cc1ed1c14072791b0bd5b9b92fa
@wchevreuil wchevreuil force-pushed the HBASE-28596-branch-2 branch from 8d90166 to 05d3993 Compare June 27, 2025 09:48
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
taklwu
taklwu approved these changes Jun 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@taklwu taklwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Apache9
Copy link
Copy Markdown
Contributor

Apache9 commented Jun 29, 2025

Is it ready to be merged?

@Apache-HBase
Copy link
Copy Markdown

Apache-HBase commented Jun 30, 2025

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 50s 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 1s The patch does not contain any @author tags.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
_ branch-2.6 Compile Tests _
+1 💚 mvninstall 4m 5s branch-2.6 passed
+1 💚 compile 2m 59s branch-2.6 passed
+1 💚 checkstyle 0m 37s branch-2.6 passed
+1 💚 spotbugs 1m 39s branch-2.6 passed
+1 💚 spotless 0m 48s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚 mvninstall 3m 10s the patch passed
+1 💚 compile 2m 58s the patch passed
+1 💚 javac 2m 58s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 0m 35s /results-checkstyle-hbase-server.txt hbase-server: The patch generated 1 new + 53 unchanged - 0 fixed = 54 total (was 53)
+1 💚 spotbugs 1m 49s the patch passed
+1 💚 hadoopcheck 17m 21s Patch does not cause any errors with Hadoop 2.10.2 or 3.3.6 3.4.0.
+1 💚 spotless 0m 44s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 11s The patch does not generate ASF License warnings.
39m 40s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6508/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #6508
Optional Tests dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname Linux 7c4ed6e0ab42 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 branch-2.6 / 05d3993
Default Java Eclipse Adoptium-11.0.23+9
Max. process+thread count 78 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6508/5/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.

@wchevreuil wchevreuil merged commit e8a3ed2 into apache:branch-2.6 Jul 1, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charlesconnell charlesconnell charlesconnell approved these changes

@taklwu taklwu taklwu approved these changes

@bbeaudreault bbeaudreault Awaiting requested review from bbeaudreault

@ndimiduk ndimiduk Awaiting requested review from ndimiduk

+1 more reviewer

@ragarkar ragarkar ragarkar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@wchevreuil @Apache-HBase @Apache9 @charlesconnell @taklwu @ragarkar