taklwu commented on code in PR #8766:
URL: https://github.com/apache/hbase/pull/8766#discussion_r4276555177
##########
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#onEvictionPressure` has parameter with
`sourceEngine` that is not being used when creating, but the `tier` is passing
to the policy.onEvictionPressure
why do we have this mis-match ? should we always use `CacheEvictionListener`
with `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]
