taklwu commented on code in PR #8575:
URL: https://github.com/apache/hbase/pull/8575#discussion_r3899514187
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java:
##########
@@ -104,12 +105,210 @@ public static TopologyBackedCacheAccessService
fromTieredExclusiveBlockCaches(St
Objects.requireNonNull(l2, "l2 must not be null");
Objects.requireNonNull(policy, "policy must not be null");
wireVictimCache(l1, l2);
- CacheEngine l1Engine = CacheEngines.fromBlockCache(l1);
+ CacheEngine l1Engine = fromL1BlockCache(l1);
CacheEngine l2Engine = CacheEngines.fromBlockCache(l2);
CacheTopology topology = new TieredExclusiveTopology(name, l1Engine,
l2Engine);
return new TopologyBackedCacheAccessService(topology, policy);
}
+ /**
+ * Creates a topology-backed cache access service for an {@link
InclusiveCombinedBlockCache}.
+ * <p>
+ * The inclusive combined cache must expose exactly two non-null legacy
block caches. The first
+ * cache is adapted as L1 using a non-victim-delegating engine, and the
second cache is adapted as
+ * L2. This prevents L1 misses from internally consulting L2 through the
legacy victim-cache
+ * mechanism and lets the topology-backed service control tier lookup and
promotion policy.
+ * </p>
+ * @param combinedBlockCache inclusive combined block cache to adapt
+ * @return topology-backed cache access service using a tiered inclusive
topology
+ * @throws NullPointerException if {@code combinedBlockCache} is {@code
null}
+ * @throws IllegalArgumentException if the combined cache does not expose
exactly two non-null
+ * block caches
+ */
+ public static TopologyBackedCacheAccessService
+ fromInclusiveCombinedBlockCache(InclusiveCombinedBlockCache
combinedBlockCache) {
+ Objects.requireNonNull(combinedBlockCache, "combinedBlockCache must not be
null");
+
+ BlockCache[] blockCaches = combinedBlockCache.getBlockCaches();
+ if (blockCaches == null || blockCaches.length != 2) {
+ throw new IllegalArgumentException(
+ "InclusiveCombinedBlockCache must expose exactly two block caches");
+ }
+ if (blockCaches[0] == null || blockCaches[1] == null) {
+ throw new IllegalArgumentException(
+ "InclusiveCombinedBlockCache must expose non-null L1 and L2 block
caches");
+ }
+
+ return fromTieredInclusiveBlockCaches("inclusive-combined",
blockCaches[0], blockCaches[1],
+ DefaultHBaseCachePlacementAdmissionPolicy.INSTANCE);
Review Comment:
can you check this one, I found InclusiveCombinedBlockCache#cacheBlock does
call
```
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean
inMemory) {
// This is the inclusive part of the combined block cache.
// Every block is placed into both block caches.
l1Cache.cacheBlock(cacheKey, buf, inMemory);
// This assumes that insertion into the L2 block cache is either async
or very fast.
l2Cache.cacheBlock(cacheKey, buf, inMemory);
}
```
does it mean the legacy `InclusiveCombinedBlockCache` should be
`SINGLE_TIER` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
