Copilot commented on code in PR #8279:
URL: https://github.com/apache/hbase/pull/8279#discussion_r3314166332
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java:
##########
@@ -417,4 +423,28 @@ public void testGetOnHeapCacheSize() {
onHeapCacheSize = MemorySizeUtil.getOnHeapCacheSize(copyConf);
assertEquals(fixedSize, onHeapCacheSize);
}
+
+ @Test
+ void testCacheAccessServiceBackedByBlockCacheWhenBlockCacheIsConfigured() {
+ Configuration conf = new Configuration(false);
+ BlockCache blockCache = BlockCacheFactory.createBlockCache(this.conf);
+ CacheConfig cacheConfig = new CacheConfig(conf, blockCache);
+
Review Comment:
In this test the `Configuration conf` used to build `CacheConfig` is
different from the configuration used to create the `BlockCache` (`this.conf`).
This makes the test harder to reason about and could hide
configuration-dependent behavior. Use a single `Configuration` instance for
both objects.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java:
##########
@@ -1104,14 +1105,14 @@ public HFileBlock getCachedBlock(BlockCacheKey
cacheKey, boolean cacheBlock, boo
boolean updateCacheMetrics, BlockType expectedBlockType,
DataBlockEncoding expectedDataBlockEncoding) throws IOException {
// Check cache for block. If found return.
- BlockCache cache = cacheConf.getBlockCache().orElse(null);
+ CacheAccessService cacheAccessService = cacheConf.getCacheAccessService();
long cachedBlockBytesRead = 0;
- if (cache != null) {
+ if (cacheAccessService != null) {
HFileBlock cachedBlock = null;
Review Comment:
`cacheConf.getCacheAccessService()` is always non-null (it is initialized to
a disabled no-op service when block cache is unavailable), so the null-check is
redundant and causes unnecessary cache lookup/try-catch overhead when caching
is disabled. Guard on `isCacheEnabled()` instead to preserve the old fast-path
when there was no cache.
--
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]
