VladRodionov commented on code in PR #8653:
URL: https://github.com/apache/hbase/pull/8653#discussion_r4067691566
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TieredInclusiveTopology.java:
##########
@@ -110,4 +110,20 @@ public void shutdown() {
l1.shutdown();
l2.shutdown();
}
+
+ /**
+ * Handles a capacity-driven eviction from this inclusive topology.
+ * <p>
+ * L1 eviction does not require demotion because inclusive placement
maintains the corresponding
+ * block in L2. L2 pressure eviction likewise does not cause movement to
another tier.
+ * </p>
+ * @param cacheKey key identifying the evicted block
+ * @param block evicted block
+ * @param sourceEngine engine that evicted the block
+ * @return {@code false}, because no additional placement is required
+ */
+ @Override
+ public boolean handleEviction(BlockCacheKey cacheKey, Cacheable block,
CacheEngine sourceEngine) {
+ return false;
Review Comment:
This race is possible, but I don't think we should add an L2 membership
lookup to every L1 pressure eviction to eliminate it. Cache placement across
independent engines is intentionally best-effort and not transactional. If L1
happens to evict before the L2 insertion is established (or an L2 insertion
fails), the consequence is only a subsequent cache miss; data correctness is
unaffected.
Checking L2 on every L1 eviction would add a cross-tier operation to the
eviction path, potentially against a much larger/slower cache, to preserve a
strict inclusion invariant that the cache does not require. I updated the
JavaDoc to make the best-effort semantics explicit. If we want to reduce this
race further, I think establishing the L2 copy before L1 during inclusive
placement would be a better approach.
--
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]
