HBASE-30329 Add topology-backed cache diagnostics and migrate single tier cache - #8575
Conversation
There was a problem hiding this comment.
Pull request overview
Routes legacy block caches and diagnostics through the topology-backed cache framework.
Changes:
- Adds single-tier and inclusive topology-backed cache support.
- Adds aggregated cached-block iteration diagnostics.
- Migrates compatibility helpers and tests to topology-backed services.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
TestCacheOnWrite.java |
Uses topology-aware cache unwrapping. |
TestCacheConfig.java |
Verifies topology-backed configuration. |
TestSingleTierTopologyBackedCacheAccessService.java |
Tests single-tier behavior. |
TestInclusiveCombinedBlockCacheCompatibleTopologyBackedCacheAccessService.java |
Tests inclusive topology semantics. |
TestCacheAccessServices.java |
Updates factory expectations. |
TestBlockCacheBackedCacheAccessService.java |
Removes legacy adapter tests. |
CacheAccessServiceTestFactory.java |
Unwraps topology-backed caches. |
CacheConfig.java |
Preserves single-tier legacy cache access. |
TopologyBackedCacheAccessServices.java |
Adds single-tier and inclusive factories. |
TopologyBackedCacheAccessService.java |
Adds single-tier writes and block iteration. |
SingleTierTopology.java |
Defines the single-tier topology. |
SingleEngineTopology.java |
Renames its topology type. |
DefaultHBaseCachePlacementAdmissionPolicy.java |
Updates single-tier policy handling. |
CacheTopologyType.java |
Introduces SINGLE_TIER. |
CacheAccessServices.java |
Routes all cache variants by topology. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
taklwu
left a comment
There was a problem hiding this comment.
actually we get more than just topology-backed cache diagnostics , the single-tier caches refactoring. maybe call the title as Add topology-backed cache diagnostics and migrate single tier cache
8d34adb to
1fb730d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/NonVictimDelegatingFirstLevelCacheEngine.java:110
- The block-type-aware overload has the same non-atomic
containsBlock/getBlocksequence. A concurrent eviction can make the second call delegate to the victim cache, and the early-return miss path also omits L1 miss metrics. Route this overload through the same atomic local-only lookup mechanism as the other overload.
if (!firstLevelBlockCache.containsBlock(cacheKey)) {
return null;
}
return super.getBlock(cacheKey, caching, repeat, updateCacheMetrics, blockType);
1fb730d to
a2fd8b5
Compare
a2fd8b5 to
e08d00d
Compare
Summary
This PR extends the topology-backed cache access framework so that cache diagnostics and cache
access are consistently routed through
TopologyBackedCacheAccessService.The main change is that plain single-tier
BlockCacheimplementations now use the sametopology-backed access path as combined caches. Single-tier caches are represented by
SingleTierTopology, while existing two-tier cache variants continue to use topology-specificrepresentations:
BlockCache->SingleTierTopologyCombinedBlockCache->TieredExclusiveTopologyInclusiveCombinedBlockCache->TieredInclusiveTopologyThis keeps the legacy cache implementations underneath through
BlockCacheBackedCacheEngine, butmoves the access-service boundary to the topology/cache-engine model.
Motivation
After HBASE-30305, exclusive combined-cache orchestration was moved behind
TopologyBackedCacheAccessService. Follow-up testing showed that diagnostic and compatibility pathsstill had assumptions about
BlockCacheBackedCacheAccessServiceand concreteBlockCacheimplementations.
This PR removes those assumptions from the main factory path by making topology-backed access the
normal path for both single-tier and combined caches.
Changes
SingleTierTopology.CacheTopologyType.SINGLE_TIER.BlockCacheinstances.InclusiveCombinedBlockCache.cacheBlock(key, block)behavior for single-tier caches.BlockCacheinstances from topology-backed serviceswhere tests still need direct compatibility checks.
Compatibility notes
BlockCacheBackedCacheAccessServiceis no longer the default factory result for plain block caches.Instead, the main factory now returns a
TopologyBackedCacheAccessServicebacked bySingleTierTopology.The underlying legacy
BlockCacheimplementations are still preserved throughBlockCacheBackedCacheEngine. This PR does not migrateLruBlockCache,BucketCache, or otherconcrete cache implementations to native
CacheEngineimplementations yet. That remains a follow-upmigration step.
Testing
Focused tests: