VladRodionov commented on code in PR #8575:
URL: https://github.com/apache/hbase/pull/8575#discussion_r3900060580
##########
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:
Yes, you found a bug, but the answer to your question is - no, it does not.
I fixed default placement policy class to handle TieredInclusivePolicy
correctly. Now it selects both tiers for write operations.
--
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]
