Copilot commented on code in PR #8697:
URL: https://github.com/apache/hbase/pull/8697#discussion_r4097026724
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TieredInclusiveTopology.java:
##########
@@ -133,4 +133,20 @@ public void shutdown() {
public boolean handleEviction(BlockCacheKey cacheKey, Cacheable block,
CacheEngine sourceEngine) {
return false;
}
+
+ /**
+ * Handles an access to a block found in this inclusive topology.
+ * <p>
+ * A block found in L1 is also expected to be present in L2. Notify L2 about
the access so that it
+ * can update any access-based metadata without reading the block. An L2 hit
requires no
+ * additional notification because L2 observed the access directly.
+ * @param cacheKey key identifying the accessed block
+ * @param sourceEngine engine in which the block was found
+ */
+ @Override
+ public void handleAccess(BlockCacheKey cacheKey, CacheEngine sourceEngine) {
+ if (sourceEngine == l1) {
+ l2.touch(cacheKey);
Review Comment:
This notification is ineffective for the built-in inclusive path: the
factory wraps legacy L2 `BlockCache` instances in
`BlockCacheBackedCacheEngine`, which inherits the new default no-op `touch`,
and the native `LruCacheEngine` also has no override. As a result, an L1 hit
calls this method but does not update L2 access metadata, so the stated
inclusive-tier use case is not actually implemented. Please wire `touch`
through the concrete/adapted engines (or explicitly defer this topology until
an engine supports it) and add an integration test through the factory.
--
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]
