Skip to content

HBASE-30305 Replace CombinedBlockCache orchestration with TieredExclusiveTopology-backed CacheAccessService - #8522

Open
VladRodionov wants to merge 1 commit into
apache:HBASE-30018from
VladRodionov:HBASE-30305-replace-combinedblockcache-orchestration
Open

HBASE-30305 Replace CombinedBlockCache orchestration with TieredExclusiveTopology-backed CacheAccessService#8522
VladRodionov wants to merge 1 commit into
apache:HBASE-30018from
VladRodionov:HBASE-30305-replace-combinedblockcache-orchestration

Conversation

@VladRodionov

Copy link
Copy Markdown
Contributor

Summary

This PR replaces CombinedBlockCache as the primary CacheAccessService orchestration path for combined L1/L2 block cache access.

The previous compatibility path exposed a CombinedBlockCache through BlockCacheBackedCacheAccessService. With the topology-backed cache architecture in place, the combined cache path can now be represented as:

CacheAccessService
  -> TopologyBackedCacheAccessService
      -> TieredExclusiveTopology
          -> BlockCacheBackedCacheEngine(L1)
          -> BlockCacheBackedCacheEngine(L2)

CombinedBlockCache is not removed in this PR. It remains available as a legacy BlockCache object and for code paths that still depend on the old API. This PR moves the migration-facing cache access path to TopologyBackedCacheAccessService.

Changes

  • Updates CacheAccessServices.fromBlockCache(...) so CombinedBlockCache is exposed as a topology-backed CacheAccessService.
  • Uses TopologyBackedCacheAccessServices.fromCombinedBlockCache(...) to extract L1/L2 caches from CombinedBlockCache and assemble a TieredExclusiveTopology.
  • Preserves regular BlockCache behavior by continuing to use BlockCacheBackedCacheAccessService for non-combined caches.
  • Extends topology-backed compatibility behavior needed by existing HFile and data-tiering paths, including:
    • combined-cache detection through the active cache service;
    • shouldCacheFile(...);
    • shouldCacheBlock(...);
    • fully cached file visibility;
    • file caching completion notification propagation.
  • Updates tests to validate the topology-backed combined-cache path.

Why this is needed

The pluggable block cache architecture is moving from implicit orchestration inside CombinedBlockCache toward explicit cache topology.

Legacy path:

LruBlockCache + BucketCache
  -> CombinedBlockCache
      -> BlockCacheBackedCacheAccessService

New path:

LruBlockCache + BucketCache
  -> BlockCacheBackedCacheEngine
      -> TieredExclusiveTopology
          -> TopologyBackedCacheAccessService

This is the next migration step after:

  • HBASE-30024, which added BlockCacheBackedCacheEngine
  • HBASE-30025, which added the helper for constructing a topology-backed combined-cache service

Compatibility notes

This PR does not remove CombinedBlockCache.

Existing concrete caches such as LruBlockCache and BucketCache still implement BlockCache. They are adapted to CacheEngine through BlockCacheBackedCacheEngine.

The goal is to move orchestration to the topology-backed access path while keeping existing cache implementations and legacy APIs intact.

Out of scope

  • No removal of CombinedBlockCache.
  • No direct migration of LruBlockCache, BucketCache, TinyLfuBlockCache, or LruAdaptiveBlockCache to implement CacheEngine.
  • No removal of BlockCacheBackedCacheAccessService.
  • No JSP/admin diagnostics cleanup.
  • No broad metrics refactoring.
  • No intended change to cache placement or eviction behavior.

Testing

Ran:

mvn -pl hbase-server -DskipTests compile

mvn -pl hbase-server -Dtest=TestHFile test
mvn -pl hbase-server -Dtest=TestDataTieringManager test
mvn -pl hbase-server -Dtest=TestCacheAccessServices test
mvn -pl hbase-server -Dtest=TestTopologyBackedCacheAccessServices test
mvn -pl hbase-server \
  -Dtest=TestCombinedBlockCacheCompatibleTopologyBackedCacheAccessService test

mvn -pl hbase-server spotless:check
mvn -pl hbase-server checkstyle:check

AI assistance disclosure

This PR was prepared with assistance from ChatGPT. All changes were reviewed, tested, and submitted by the author.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR advances the block-cache migration in hbase-server by switching the combined L1/L2 orchestration path from the legacy CombinedBlockCache-wrapped BlockCacheBackedCacheAccessService to a topology-driven TopologyBackedCacheAccessService using TieredExclusiveTopology, while keeping CombinedBlockCache available for legacy BlockCache APIs.

Changes:

  • Update CacheAccessServices.fromBlockCache(...) to return a topology-backed cache access service when the input is a CombinedBlockCache.
  • Add a TopologyBackedCacheAccessServices.fromCombinedBlockCache(...) construction path that extracts L1/L2 block caches and assembles a TieredExclusiveTopology.
  • Extend compatibility APIs (e.g., shouldCacheFile, shouldCacheBlock, fully-cached file visibility, completion notifications) and add/adjust tests for the new wiring.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java Adds diagnostics around fully-cached-file waiting; removes an unused helper.
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java Adjusts combined-cache test setup (currently by commenting an assertion).
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServices.java Adds tests for constructing a topology-backed service from CombinedBlockCache.
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCacheAccessServices.java Adds coverage for CacheAccessServices.fromBlockCache(...) behavior (regular vs combined).
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java Enhances “combined cache” detection to include topology-backed compatibility.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java Adds fromCombinedBlockCache(...) helpers for wiring L1/L2 into TieredExclusiveTopology.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java Implements combined-cache compatibility behaviors across engines and propagates notifications.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheEngine.java Adds default optional hooks for file/block admission and fully-cached-file reporting.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java Routes CombinedBlockCache to topology-backed service construction.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessService.java Adds null checks, propagates block category in default cache writes, and adds getFullyCachedFiles().
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheEngine.java Delegates new compatibility hooks to the underlying legacy BlockCache.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheAccessService.java Exposes getFullyCachedFiles() through the legacy-backed service.
hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureBypass.java Whitespace-only change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java Outdated

@taklwu taklwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you run mvn spotless:apply and fix the spotless first ?

@taklwu taklwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

below tests are failing, can you check ?
TestCacheConfig.testBucketCacheConfigL1L2Setup
TestCacheConfig.testFileBucketCacheConfig
TestCacheConfig.testOffHeapBucketCacheConfig

Comment thread hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java Outdated
@VladRodionov
VladRodionov force-pushed the HBASE-30305-replace-combinedblockcache-orchestration branch from 945f39d to d283f21 Compare August 6, 2026 01:07
@taklwu

taklwu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

spotless is still failing, can you check ?

TestAvoidCellReferencesIntoShippedBlocks.testHBASE16372InReadPath failed, can you try if that is failing in your end?

TestProcDispatcher failed but it should be unrelated, let's wait for your next commit and try again.

@VladRodionov

Copy link
Copy Markdown
Contributor Author

I routinely run

mvn -pl hbase-server spotless:apply
mvn -pl hbase-server spotless:check

This is probably checkstyle error, which spotless can catch. Will double check. Checkstyle outputs a lot of errors, it's hard to catch which one you are responsible for.

@taklwu

taklwu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

I routinely run

mvn -pl hbase-server spotless:apply
mvn -pl hbase-server spotless:check

This is probably checkstyle error, which spotless can catch. Will double check. Checkstyle outputs a lot of errors, it's hard to catch which one you are responsible for.

I thought you have the access to download the result, where the patch-spotless.txt shows the error is from TopologyBackedCacheAccessService

[ERROR] Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.44.4:check (default-cli) on project hbase-server: The following files had format violations:
[ERROR]     src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java
[ERROR]         @@ -32,7 +32,6 @@
[ERROR]          import·org.apache.hadoop.hbase.util.Pair;
[ERROR]          import·org.apache.yetus.audience.InterfaceAudience;
[ERROR]          
[ERROR]         -
[ERROR]          /**
[ERROR]          ·*·{@link·CacheAccessService}·implementation·backed·by·{@link·CacheTopology}·and·{@link·CacheEngine}
[ERROR]          ·*·instances.
[ERROR] Run 'mvn spotless:apply' to fix these violations.
[ERROR] -> [Help 1]

I recalled running mvn spotless:apply at the root level would fix the problem, doesn't it ?

there is another checkstyle error

./hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java:317:    ;:5: Empty statement. [EmptyStatement]

Sign up for free to 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.

3 participants