VladRodionov commented on code in PR #8766:
URL: https://github.com/apache/hbase/pull/8766#discussion_r4291407463
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java:
##########
@@ -79,11 +79,48 @@ public TopologyBackedCacheAccessService(CacheTopology
topology,
Objects.requireNonNull(topology.getView(), "topology view must not be
null");
for (CacheTier tier : topology.getTiers()) {
topology.getEngine(tier)
- .ifPresent(engine ->
engine.setEvictionListener(this::handleEngineEviction));
+ .ifPresent(engine ->
engine.setEvictionListener(createEvictionListener(tier)));
}
this.persistenceCoordinator = new CachePersistenceCoordinator(topology,
policy);
}
+ /**
+ * Creates an eviction listener associated with the specified cache tier.
+ * <p>
+ * Per-block eviction events are forwarded to the topology for
topology-specific handling.
+ * Eviction-pressure events are forwarded to the placement/admission policy.
+ * </p>
+ * @param tier tier associated with the cache engine
+ * @return eviction listener for the tier
+ */
+ private CacheEvictionListener createEvictionListener(CacheTier tier) {
+ Objects.requireNonNull(tier, "tier must not be null");
+
+ return new CacheEvictionListener() {
+
+ /**
+ * Handles a capacity-driven block eviction.
+ * @param sourceEngine engine that evicted the block
+ * @param cacheKey key identifying the evicted block
+ * @param block evicted block
+ */
+ @Override
+ public void onEviction(CacheEngine sourceEngine, BlockCacheKey cacheKey,
Cacheable block) {
+ handleEngineEviction(sourceEngine, cacheKey, block);
+ }
+
+ /**
+ * Handles cache pressure reported by an eviction pass.
+ * @param sourceEngine engine that performed the eviction pass
+ * @param bytesToFree amount of cache pressure observed, in bytes
+ */
+ @Override
+ public void onEvictionPressure(CacheEngine sourceEngine, long
bytesToFree) {
Review Comment:
CacheEvictionListener is an engine-level API, so it reports the CacheEngine
that generated the event. The engine does not know whether it is used as
SINGLE, L1, or L2.
TopologyBackedCacheAccessService knows that mapping, so when it registers
the listener it captures the corresponding CacheTier and passes that tier to
the policy.
So the mismatch is intentional: engine event in, topology tier out. I would
keep CacheEvictionListener independent of CacheTier.
--
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]
