This is an automated email from the ASF dual-hosted git repository.
taklwu pushed a commit to branch HBASE-30018
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/HBASE-30018 by this push:
new 8d8bf5449d0 HBASE-30022 Refactor CacheConfig and HFileReaderImpl to
use CacheAccessService (#8279)
8d8bf5449d0 is described below
commit 8d8bf5449d0610f00b39dd2aa9776611ef320542
Author: Vladimir Rodionov <[email protected]>
AuthorDate: Thu May 28 15:01:58 2026 -0700
HBASE-30022 Refactor CacheConfig and HFileReaderImpl to use
CacheAccessService (#8279)
Signed-off-by: Wellington Chevreuil <[email protected]>
Signed-off-by: Tak Lon (Stephen) Wu <[email protected]>
Reviewed by: Kota-SH <[email protected]>
---
.../apache/hadoop/hbase/io/hfile/CacheConfig.java | 25 +++
.../hadoop/hbase/io/hfile/HFilePreadReader.java | 16 +-
.../hadoop/hbase/io/hfile/HFileReaderImpl.java | 200 ++++++++++-----------
.../cache/BlockCacheBackedCacheAccessService.java | 8 +
.../hbase/io/hfile/cache/CacheAccessService.java | 24 +++
.../hadoop/hbase/io/hfile/cache/CacheEngine.java | 13 ++
.../hadoop/hbase/io/hfile/cache/CacheTopology.java | 1 +
.../io/hfile/cache/NoOpCacheAccessService.java | 7 +
.../hadoop/hbase/io/hfile/TestCacheConfig.java | 32 +++-
.../TestBlockCacheBackedCacheAccessService.java | 25 +++
.../io/hfile/cache/TestNoOpCacheAccessService.java | 117 ++++++++++++
11 files changed, 360 insertions(+), 108 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
index 475a22703e1..3505f822814 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
@@ -24,6 +24,8 @@ import org.apache.hadoop.hbase.conf.ConfigurationManager;
import org.apache.hadoop.hbase.conf.PropagatingConfigurationObserver;
import org.apache.hadoop.hbase.io.ByteBuffAllocator;
import org.apache.hadoop.hbase.io.hfile.BlockType.BlockCategory;
+import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService;
+import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessServices;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -176,6 +178,8 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
// Local reference to the block cache
private final BlockCache blockCache;
+ private final CacheAccessService cacheAccessService;
+
private final ByteBuffAllocator byteBuffAllocator;
private double heapUsageThreshold;
@@ -231,6 +235,9 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
}
this.blockCache = blockCache;
this.byteBuffAllocator = byteBuffAllocator;
+ this.cacheAccessService = blockCache != null
+ ? CacheAccessServices.fromBlockCache(blockCache)
+ : CacheAccessServices.disabled();
}
/**
@@ -252,6 +259,9 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
this.blockCache = cacheConf.blockCache;
this.byteBuffAllocator = cacheConf.byteBuffAllocator;
this.heapUsageThreshold = cacheConf.heapUsageThreshold;
+ this.cacheAccessService = blockCache != null
+ ? CacheAccessServices.fromBlockCache(blockCache)
+ : CacheAccessServices.disabled();
}
private CacheConfig() {
@@ -270,6 +280,7 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
this.blockCache = null;
this.byteBuffAllocator = ByteBuffAllocator.HEAP;
this.heapUsageThreshold = DEFAULT_PREFETCH_HEAP_USAGE_THRESHOLD;
+ this.cacheAccessService = CacheAccessServices.disabled();
}
/**
@@ -469,6 +480,20 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
return Optional.ofNullable(this.blockCache);
}
+ /**
+ * Returns the cache access service used by HFile read/write path callers.
+ * <p>
+ * This service is the migration-facing cache abstraction. For now it is
backed by the existing
+ * {@link BlockCache} when block cache is configured, or by a disabled no-op
implementation when
+ * block cache is unavailable. This keeps cache construction unchanged while
allowing callers such
+ * as {@code HFileReaderImpl} to depend on {@link CacheAccessService}.
+ * </p>
+ * @return cache access service
+ */
+ public CacheAccessService getCacheAccessService() {
+ return cacheAccessService;
+ }
+
public boolean isCombinedBlockCache() {
return blockCache instanceof CombinedBlockCache;
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java
index 72e6cc1807a..2d4a0174517 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePreadReader.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper;
+import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -65,7 +66,7 @@ public class HFilePreadReader extends HFileReaderImpl {
// Don't use BlockIterator here, because it's designed to read
load-on-open section.
long onDiskSizeOfNextBlock = -1;
// if we are here, block cache is present anyways
- BlockCache cache = cacheConf.getBlockCache().get();
+ CacheAccessService cacheAccessService =
cacheConf.getCacheAccessService();
boolean interrupted = false;
int blockCount = 0;
int dataBlockCount = 0;
@@ -78,10 +79,10 @@ public class HFilePreadReader extends HFileReaderImpl {
// update the offset and move on to the next block without
actually going read all
// the way to the cache.
BlockCacheKey cacheKey = new BlockCacheKey(name, offset);
- if (cache.isAlreadyCached(cacheKey).orElse(false)) {
+ if (cacheAccessService.isAlreadyCached(cacheKey).orElse(false)) {
// Right now, isAlreadyCached is only supported by
BucketCache, which should
// always cache data blocks.
- int size = cache.getBlockSize(cacheKey).orElse(0);
+ int size = cacheAccessService.getBlockSize(cacheKey).orElse(0);
if (size > 0) {
offset += size;
LOG.debug("Found block of size {} for cache key {}. "
@@ -108,11 +109,12 @@ public class HFilePreadReader extends HFileReaderImpl {
/* cacheBlock= */true, /* pread= */false, false, false, null,
null, true);
try {
if (!cacheConf.isInMemory()) {
- if (!cache.blockFitsIntoTheCache(block).orElse(true)) {
+ if
(!cacheAccessService.blockFitsIntoTheCache(block).orElse(true)) {
LOG.warn(
"Interrupting prefetch for file {} because block {} of
size {} "
+ "doesn't fit in the available cache space.
isCacheEnabled: {}",
- path, cacheKey, block.getOnDiskSizeWithHeader(),
cache.isCacheEnabled());
+ path, cacheKey, block.getOnDiskSizeWithHeader(),
+ cacheAccessService.isCacheEnabled());
interrupted = true;
break;
}
@@ -139,8 +141,8 @@ public class HFilePreadReader extends HFileReaderImpl {
}
}
if (!interrupted) {
- cacheConf.getBlockCache().get().notifyFileCachingCompleted(path,
blockCount,
- dataBlockCount, offset);
+ cacheAccessService.notifyFileCachingCompleted(path, blockCount,
dataBlockCount,
+ offset);
}
} catch (IOException e) {
// IOExceptions are probably due to region closes (relocation,
etc.)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
index 6e8f11711ae..6e09e0ba345 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
@@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.io.encoding.DataBlockEncoder;
import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
import org.apache.hadoop.hbase.io.encoding.HFileBlockDecodingContext;
+import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService;
import org.apache.hadoop.hbase.monitoring.ThreadLocalServerSideScanMetrics;
import org.apache.hadoop.hbase.nio.ByteBuff;
import org.apache.hadoop.hbase.regionserver.KeyValueScanner;
@@ -1104,120 +1105,120 @@ public abstract class HFileReaderImpl implements
HFile.Reader, Configurable {
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) {
- HFileBlock cachedBlock = null;
- boolean isScanMetricsEnabled =
ThreadLocalServerSideScanMetrics.isScanMetricsEnabled();
- try {
- cachedBlock = (HFileBlock) cache.getBlock(cacheKey, cacheBlock,
useLock, updateCacheMetrics,
- expectedBlockType);
- if (cachedBlock != null) {
- if
(cacheConf.shouldCacheCompressed(cachedBlock.getBlockType().getCategory())) {
- HFileBlock compressedBlock = cachedBlock;
- cachedBlock = compressedBlock.unpack(hfileContext, fsBlockReader);
- // In case of compressed block after unpacking we can release the
compressed block
- if (compressedBlock != cachedBlock) {
- compressedBlock.release();
- }
- }
- try {
- validateBlockType(cachedBlock, expectedBlockType);
- } catch (IOException e) {
- returnAndEvictBlock(cache, cacheKey, cachedBlock);
- cachedBlock = null;
- throw e;
+ HFileBlock cachedBlock = null;
+ boolean isScanMetricsEnabled =
ThreadLocalServerSideScanMetrics.isScanMetricsEnabled();
+ try {
+ cachedBlock = (HFileBlock) cacheAccessService.getBlock(cacheKey,
cacheBlock, useLock,
+ updateCacheMetrics, expectedBlockType);
+ if (cachedBlock != null) {
+ if
(cacheConf.shouldCacheCompressed(cachedBlock.getBlockType().getCategory())) {
+ HFileBlock compressedBlock = cachedBlock;
+ cachedBlock = compressedBlock.unpack(hfileContext, fsBlockReader);
+ // In case of compressed block after unpacking we can release the
compressed block
+ if (compressedBlock != cachedBlock) {
+ compressedBlock.release();
}
+ }
+ try {
+ validateBlockType(cachedBlock, expectedBlockType);
+ } catch (IOException e) {
+ returnAndEvictBlock(cacheAccessService, cacheKey, cachedBlock);
+ cachedBlock = null;
+ throw e;
+ }
- if (expectedDataBlockEncoding == null) {
- return cachedBlock;
- }
- DataBlockEncoding actualDataBlockEncoding =
cachedBlock.getDataBlockEncoding();
- // Block types other than data blocks always have
- // DataBlockEncoding.NONE. To avoid false negative cache misses, only
- // perform this check if cached block is a data block.
+ if (expectedDataBlockEncoding == null) {
+ return cachedBlock;
+ }
+ DataBlockEncoding actualDataBlockEncoding =
cachedBlock.getDataBlockEncoding();
+ // Block types other than data blocks always have
+ // DataBlockEncoding.NONE. To avoid false negative cache misses, only
+ // perform this check if cached block is a data block.
+ if (
+ cachedBlock.getBlockType().isData()
+ && !actualDataBlockEncoding.equals(expectedDataBlockEncoding)
+ ) {
+ // This mismatch may happen if a Scanner, which is used for say a
+ // compaction, tries to read an encoded block from the block cache.
+ // The reverse might happen when an EncodedScanner tries to read
+ // un-encoded blocks which were cached earlier.
+ //
+ // Because returning a data block with an implicit BlockType mismatch
+ // will cause the requesting scanner to throw a disk read should be
+ // forced here. This will potentially cause a significant number of
+ // cache misses, so update so we should keep track of this as it
might
+ // justify the work on a CompoundScanner.
if (
- cachedBlock.getBlockType().isData()
- && !actualDataBlockEncoding.equals(expectedDataBlockEncoding)
+ !expectedDataBlockEncoding.equals(DataBlockEncoding.NONE)
+ && !actualDataBlockEncoding.equals(DataBlockEncoding.NONE)
) {
- // This mismatch may happen if a Scanner, which is used for say a
- // compaction, tries to read an encoded block from the block cache.
- // The reverse might happen when an EncodedScanner tries to read
- // un-encoded blocks which were cached earlier.
- //
- // Because returning a data block with an implicit BlockType
mismatch
- // will cause the requesting scanner to throw a disk read should be
- // forced here. This will potentially cause a significant number of
- // cache misses, so update so we should keep track of this as it
might
- // justify the work on a CompoundScanner.
- if (
- !expectedDataBlockEncoding.equals(DataBlockEncoding.NONE)
- && !actualDataBlockEncoding.equals(DataBlockEncoding.NONE)
- ) {
- // If the block is encoded but the encoding does not match the
- // expected encoding it is likely the encoding was changed but
the
- // block was not yet evicted. Evictions on file close happen
async
- // so blocks with the old encoding still linger in cache for some
- // period of time. This event should be rare as it only happens
on
- // schema definition change.
- LOG.info(
- "Evicting cached block with key {} because data block encoding
mismatch; "
- + "expected {}, actual {}, path={}",
- cacheKey, actualDataBlockEncoding, expectedDataBlockEncoding,
path);
- // This is an error scenario. so here we need to release the
block.
- returnAndEvictBlock(cache, cacheKey, cachedBlock);
- }
- cachedBlock = null;
- return null;
+ // If the block is encoded but the encoding does not match the
+ // expected encoding it is likely the encoding was changed but the
+ // block was not yet evicted. Evictions on file close happen async
+ // so blocks with the old encoding still linger in cache for some
+ // period of time. This event should be rare as it only happens on
+ // schema definition change.
+ LOG.info(
+ "Evicting cached block with key {} because data block encoding
mismatch; "
+ + "expected {}, actual {}, path={}",
+ cacheKey, actualDataBlockEncoding, expectedDataBlockEncoding,
path);
+ // This is an error scenario. so here we need to release the block.
+ returnAndEvictBlock(cacheAccessService, cacheKey, cachedBlock);
}
- return cachedBlock;
- }
- } catch (Exception e) {
- if (cachedBlock != null) {
- returnAndEvictBlock(cache, cacheKey, cachedBlock);
+ cachedBlock = null;
+ return null;
}
- LOG.warn("Failed retrieving block from cache with key {}. "
- + "\n Evicting this block from cache and will read it from file
system. "
- + "\n Exception details: ", cacheKey, e);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Further tracing details for failed block cache retrieval:"
+ return cachedBlock;
+ }
+ } catch (Exception e) {
+ if (cachedBlock != null) {
+ returnAndEvictBlock(cacheAccessService, cacheKey, cachedBlock);
+ }
+ LOG.warn("Failed retrieving block from cache with key {}. "
+ + "\n Evicting this block from cache and will read it from file
system. "
+ + "\n Exception details: ", cacheKey, e);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(
+ "Further tracing details for failed block cache retrieval:"
+ "\n Complete File path - {}," + "\n Expected Block Type - {},
Actual Block Type - {},"
+ "\n Cache compressed - {}" + "\n Header size (after deserialized
from cache) - {}"
+ "\n Size with header - {}" + "\n Uncompressed size without
header - {} "
- + "\n Total byte buffer size - {}" + "\n Encoding code - {}",
this.path,
- expectedBlockType, (cachedBlock != null ?
cachedBlock.getBlockType() : "N/A"),
- (expectedBlockType != null
- ?
cacheConf.shouldCacheCompressed(expectedBlockType.getCategory())
- : "N/A"),
- (cachedBlock != null ? cachedBlock.headerSize() : "N/A"),
- (cachedBlock != null ? cachedBlock.getOnDiskSizeWithHeader() :
"N/A"),
- (cachedBlock != null ?
cachedBlock.getUncompressedSizeWithoutHeader() : "N/A"),
- (cachedBlock != null ? cachedBlock.getBufferReadOnly().limit() :
"N/A"),
- (cachedBlock != null
- ?
cachedBlock.getBufferReadOnly().getShort(cachedBlock.headerSize())
- : "N/A"));
- }
- return null;
- } finally {
- // Count bytes read as cached block is being returned
- if (isScanMetricsEnabled && cachedBlock != null) {
- cachedBlockBytesRead = cachedBlock.getOnDiskSizeWithHeader();
- // Account for the header size of the next block if it exists
- if (cachedBlock.getNextBlockOnDiskSize() > 0) {
- cachedBlockBytesRead += cachedBlock.headerSize();
- }
- }
- if (cachedBlockBytesRead > 0) {
-
ThreadLocalServerSideScanMetrics.addBytesReadFromBlockCache(cachedBlockBytesRead);
+ + "\n Total byte buffer size - {}" + "\n Encoding code - {}",
+ this.path, expectedBlockType, (cachedBlock != null ?
cachedBlock.getBlockType() : "N/A"),
+ (expectedBlockType != null
+ ? cacheConf.shouldCacheCompressed(expectedBlockType.getCategory())
+ : "N/A"),
+ (cachedBlock != null ? cachedBlock.headerSize() : "N/A"),
+ (cachedBlock != null ? cachedBlock.getOnDiskSizeWithHeader() :
"N/A"),
+ (cachedBlock != null ?
cachedBlock.getUncompressedSizeWithoutHeader() : "N/A"),
+ (cachedBlock != null ? cachedBlock.getBufferReadOnly().limit() :
"N/A"),
+ (cachedBlock != null
+ ?
cachedBlock.getBufferReadOnly().getShort(cachedBlock.headerSize())
+ : "N/A"));
+ }
+ return null;
+ } finally {
+ // Count bytes read as cached block is being returned
+ if (isScanMetricsEnabled && cachedBlock != null) {
+ cachedBlockBytesRead = cachedBlock.getOnDiskSizeWithHeader();
+ // Account for the header size of the next block if it exists
+ if (cachedBlock.getNextBlockOnDiskSize() > 0) {
+ cachedBlockBytesRead += cachedBlock.headerSize();
}
}
+ if (cachedBlockBytesRead > 0) {
+
ThreadLocalServerSideScanMetrics.addBytesReadFromBlockCache(cachedBlockBytesRead);
+ }
}
return null;
}
- private void returnAndEvictBlock(BlockCache cache, BlockCacheKey cacheKey,
Cacheable block) {
+ private void returnAndEvictBlock(CacheAccessService cacheAccessService,
BlockCacheKey cacheKey,
+ Cacheable block) {
block.release();
- cache.evictBlock(cacheKey);
+ cacheAccessService.evictBlock(cacheKey);
}
/**
@@ -1373,9 +1374,8 @@ public abstract class HFileReaderImpl implements
HFile.Reader, Configurable {
// type in the cache key, and we expect it to match on a cache
hit.
if (cachedBlock.getDataBlockEncoding() !=
dataBlockEncoder.getDataBlockEncoding()) {
// Remember to release the block when in exceptional path.
- cacheConf.getBlockCache().ifPresent(cache -> {
- returnAndEvictBlock(cache, cacheKey, cachedBlock);
- });
+ CacheAccessService cacheAccessService =
cacheConf.getCacheAccessService();
+ returnAndEvictBlock(cacheAccessService, cacheKey, cachedBlock);
throw new IOException("Cached block under key " + cacheKey + "
"
+ "has wrong encoding: " +
cachedBlock.getDataBlockEncoding() + " (expected: "
+ dataBlockEncoder.getDataBlockEncoding() + "), path=" +
path);
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheAccessService.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheAccessService.java
index 4b3d0ef6566..32c012c96f8 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheAccessService.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheAccessService.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.io.hfile.cache;
import java.util.Objects;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.hfile.BlockCache;
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
import org.apache.hadoop.hbase.io.hfile.BlockType;
@@ -297,4 +298,11 @@ public class BlockCacheBackedCacheAccessService implements
CacheAccessService {
Objects.requireNonNull(config, "config must not be null");
blockCache.onConfigurationChange(config);
}
+
+ @Override
+ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount,
int dataBlockCount,
+ long size) {
+ Objects.requireNonNull(fileName, "fileName must not be null");
+ blockCache.notifyFileCachingCompleted(fileName, totalBlockCount,
dataBlockCount, size);
+ }
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessService.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessService.java
index ab258eabc14..b4660a71dbf 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessService.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessService.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.io.hfile.cache;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
import org.apache.hadoop.hbase.io.hfile.BlockType;
import org.apache.hadoop.hbase.io.hfile.CacheStats;
@@ -420,4 +421,27 @@ public interface CacheAccessService {
default void onConfigurationChange(Configuration config) {
// noop
}
+
+ /**
+ * Notifies the cache service that cache population for an HFile has
completed.
+ * <p>
+ * This callback is used for file-level cache lifecycle notifications. Some
cache implementations
+ * may use it to finalize file-scoped metadata, update fully-cached-file
tracking, publish cache
+ * population statistics, or trigger implementation-specific bookkeeping
after a writer/prefetcher
+ * has finished caching blocks for an HFile.
+ * </p>
+ * <p>
+ * The default implementation is a no-op because not all cache
implementations need
+ * file-completion notifications. Callers may invoke this method
unconditionally after file-level
+ * cache population completes.
+ * </p>
+ * @param fileName path of the HFile whose cache population completed
+ * @param totalBlockCount total number of cached blocks for the file
+ * @param dataBlockCount number of cached data blocks for the file
+ * @param size total cached size for the file, in bytes
+ */
+ default void notifyFileCachingCompleted(Path fileName, int totalBlockCount,
int dataBlockCount,
+ long size) {
+ // noop
+ }
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheEngine.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheEngine.java
index 8d6629de3e2..b5564657dfb 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheEngine.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheEngine.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.io.hfile.cache;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
import org.apache.hadoop.hbase.io.hfile.BlockType;
import org.apache.hadoop.hbase.io.hfile.CacheStats;
@@ -289,4 +290,16 @@ public interface CacheEngine {
default void onConfigurationChange(Configuration config) {
// noop
}
+
+ /**
+ * Notifies the cache service that cache population for an HFile has
completed.
+ * @param fileName path of the HFile whose cache population completed
+ * @param totalBlockCount total number of cached blocks for the file
+ * @param dataBlockCount number of cached data blocks for the file
+ * @param size total cached size for the file, in bytes
+ */
+ default void notifyFileCachingCompleted(Path fileName, int totalBlockCount,
int dataBlockCount,
+ long size) {
+ // noop
+ }
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheTopology.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheTopology.java
index daf7a053ea4..16e13914873 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheTopology.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheTopology.java
@@ -198,4 +198,5 @@ public interface CacheTopology {
* @return read-only topology view
*/
CacheTopologyView getView();
+
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/NoOpCacheAccessService.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/NoOpCacheAccessService.java
index 570ab93e8b2..5c9da57aa10 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/NoOpCacheAccessService.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/NoOpCacheAccessService.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.io.hfile.cache;
import java.util.Objects;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
import org.apache.hadoop.hbase.io.hfile.CacheStats;
import org.apache.hadoop.hbase.io.hfile.Cacheable;
@@ -283,4 +284,10 @@ public final class NoOpCacheAccessService implements
CacheAccessService {
public void onConfigurationChange(Configuration config) {
Objects.requireNonNull(config, "config must not be null");
}
+
+ @Override
+ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount,
int dataBlockCount,
+ long size) {
+ Objects.requireNonNull(fileName, "fileName must not be null");
+ }
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java
index c85ed803f11..eea755218b6 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheConfig.java
@@ -17,8 +17,10 @@
*/
package org.apache.hadoop.hbase.io.hfile;
+import static org.junit.Assert.assertSame;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -37,6 +39,9 @@ import
org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.io.ByteBuffAllocator;
import org.apache.hadoop.hbase.io.hfile.BlockType.BlockCategory;
import org.apache.hadoop.hbase.io.hfile.bucket.BucketCache;
+import
org.apache.hadoop.hbase.io.hfile.cache.BlockCacheBackedCacheAccessService;
+import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService;
+import org.apache.hadoop.hbase.io.hfile.cache.NoOpCacheAccessService;
import org.apache.hadoop.hbase.io.util.MemorySizeUtil;
import org.apache.hadoop.hbase.nio.ByteBuff;
import org.apache.hadoop.hbase.testclassification.IOTests;
@@ -362,8 +367,9 @@ public class TestCacheConfig {
}
});
// The eviction thread in lrublockcache needs to run.
- while (initialL1BlockCount != lbc.getBlockCount())
+ while (initialL1BlockCount != lbc.getBlockCount()) {
Threads.sleep(10);
+ }
assertEquals(initialL1BlockCount, lbc.getBlockCount());
}
@@ -417,4 +423,28 @@ public class TestCacheConfig {
onHeapCacheSize = MemorySizeUtil.getOnHeapCacheSize(copyConf);
assertEquals(fixedSize, onHeapCacheSize);
}
+
+ @Test
+ void testCacheAccessServiceBackedByBlockCacheWhenBlockCacheIsConfigured() {
+ Configuration conf = this.conf;
+ BlockCache blockCache = BlockCacheFactory.createBlockCache(conf);
+ CacheConfig cacheConfig = new CacheConfig(conf, blockCache);
+
+ CacheAccessService service = cacheConfig.getCacheAccessService();
+
+ assertInstanceOf(BlockCacheBackedCacheAccessService.class, service);
+ assertSame(blockCache, ((BlockCacheBackedCacheAccessService)
service).getBlockCache());
+ }
+
+ @Test
+ void testCacheAccessServiceIsNoOpWhenBlockCacheIsNull() {
+ Configuration conf = this.conf;
+
+ CacheConfig cacheConfig = new CacheConfig(conf, null);
+
+ CacheAccessService service = cacheConfig.getCacheAccessService();
+
+ assertInstanceOf(NoOpCacheAccessService.class, service);
+ assertFalse(service.isCacheEnabled());
+ }
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestBlockCacheBackedCacheAccessService.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestBlockCacheBackedCacheAccessService.java
index bce743aa1b5..55dd5a579db 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestBlockCacheBackedCacheAccessService.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestBlockCacheBackedCacheAccessService.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.io.hfile.cache;
+import static org.junit.Assert.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
@@ -29,6 +30,7 @@ import static org.mockito.Mockito.when;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.io.hfile.BlockCache;
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
import org.apache.hadoop.hbase.io.hfile.BlockType;
@@ -218,4 +220,27 @@ public class TestBlockCacheBackedCacheAccessService {
service.onConfigurationChange(new Configuration(false));
service.shutdown();
}
+
+ @Test
+ void testNotifyFileCachingCompletedDelegatesToBlockCache() {
+ BlockCache blockCache = mock(BlockCache.class);
+ CacheAccessService service = new
BlockCacheBackedCacheAccessService(blockCache);
+ Path fileName = new Path("/hbase/table/region/family/file");
+ int totalBlockCount = 10;
+ int dataBlockCount = 8;
+ long size = 1024L;
+
+ service.notifyFileCachingCompleted(fileName, totalBlockCount,
dataBlockCount, size);
+
+ verify(blockCache).notifyFileCachingCompleted(fileName, totalBlockCount,
dataBlockCount, size);
+ }
+
+ @Test
+ void testNotifyFileCachingCompletedRejectsNullPath() {
+ BlockCache blockCache = mock(BlockCache.class);
+ CacheAccessService service = new
BlockCacheBackedCacheAccessService(blockCache);
+
+ assertThrows(NullPointerException.class,
+ () -> service.notifyFileCachingCompleted(null, 10, 8, 1024L));
+ }
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestNoOpCacheAccessService.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestNoOpCacheAccessService.java
new file mode 100644
index 00000000000..582aa7e0241
--- /dev/null
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestNoOpCacheAccessService.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.hfile.cache;
+
+import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+
+import java.util.Optional;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
+import org.apache.hadoop.hbase.io.hfile.CacheStats;
+import org.apache.hadoop.hbase.io.hfile.Cacheable;
+import org.apache.hadoop.hbase.io.hfile.HFileBlock;
+import org.apache.hadoop.hbase.testclassification.IOTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for {@link NoOpCacheAccessService} and related service helpers.
+ */
+@Tag(IOTests.TAG)
+@Tag(SmallTests.TAG)
+
+public class TestNoOpCacheAccessService {
+
+ private static final String HFILE_NAME = "file";
+ private static final String REGION_NAME = "region";
+ private static final long BLOCK_OFFSET = 1L;
+ private static final long RANGE_START_OFFSET = 1L;
+ private static final long RANGE_END_OFFSET = 10L;
+
+ @Test
+ void testNoOpCacheAccessService() {
+ CacheAccessService service = new NoOpCacheAccessService(new
CacheStats("noop"));
+ BlockCacheKey key = new BlockCacheKey(HFILE_NAME, BLOCK_OFFSET);
+ CacheRequestContext requestContext =
CacheRequestContext.newBuilder().setCaching(true)
+ .setRepeat(false).setUpdateCacheMetrics(true).build();
+ CacheWriteContext writeContext =
CacheWriteContext.newBuilder().setInMemory(true)
+ .setWaitWhenCache(true).setSource(CacheWriteSource.READ_MISS).build();
+ Cacheable block = mock(Cacheable.class);
+ Configuration conf = new Configuration(false);
+
+ assertEquals("NoOpCacheAccessService", service.getName());
+ assertNull(service.getBlock(key, requestContext));
+
+ service.cacheBlock(key, block, writeContext);
+
+ assertFalse(service.evictBlock(key));
+ assertEquals(0, service.evictBlocksByHfileName(HFILE_NAME));
+ assertEquals(0,
+ service.evictBlocksRangeByHfileName(HFILE_NAME, RANGE_START_OFFSET,
RANGE_END_OFFSET));
+ assertEquals(0, service.evictBlocksByRegionName(REGION_NAME));
+
+ assertEquals(0L, service.getMaxSize());
+ assertEquals(0L, service.getFreeSize());
+ assertEquals(0L, service.size());
+ assertEquals(0L, service.getCurrentDataSize());
+ assertEquals(0L, service.getBlockCount());
+ assertEquals(0L, service.getDataBlockCount());
+
+ assertEquals(Optional.empty(),
service.blockFitsIntoTheCache(mock(HFileBlock.class)));
+ assertEquals(Optional.empty(), service.isAlreadyCached(key));
+ assertEquals(Optional.empty(), service.getBlockSize(key));
+
+ assertFalse(service.isCacheEnabled());
+ assertFalse(service.waitForCacheInitialization(1L));
+
+ service.onConfigurationChange(conf);
+ service.shutdown();
+ }
+
+ @Test
+ void testNoOpCacheAccessServiceRejectsNullInputs() {
+ CacheAccessService service = new NoOpCacheAccessService(new
CacheStats("noop"));
+ Cacheable block = mock(Cacheable.class);
+ CacheRequestContext requestContext =
CacheRequestContext.newBuilder().build();
+ CacheWriteContext writeContext = CacheWriteContext.newBuilder().build();
+
+ assertThrows(NullPointerException.class, () -> new
NoOpCacheAccessService(null));
+ assertThrows(NullPointerException.class, () -> service.getBlock(null,
requestContext));
+ assertThrows(NullPointerException.class,
+ () -> service.getBlock(new BlockCacheKey(HFILE_NAME, BLOCK_OFFSET),
null));
+ assertThrows(NullPointerException.class, () -> service.cacheBlock(null,
block, writeContext));
+ assertThrows(NullPointerException.class,
+ () -> service.cacheBlock(new BlockCacheKey(HFILE_NAME, BLOCK_OFFSET),
null, writeContext));
+ assertThrows(NullPointerException.class,
+ () -> service.cacheBlock(new BlockCacheKey(HFILE_NAME, BLOCK_OFFSET),
block, null));
+ assertThrows(NullPointerException.class, () -> service.evictBlock(null));
+ assertThrows(NullPointerException.class, () ->
service.evictBlocksByHfileName(null));
+ assertThrows(NullPointerException.class,
+ () -> service.evictBlocksRangeByHfileName(null, RANGE_START_OFFSET,
RANGE_END_OFFSET));
+ assertThrows(NullPointerException.class, () ->
service.evictBlocksByRegionName(null));
+ assertThrows(NullPointerException.class, () ->
service.blockFitsIntoTheCache(null));
+ assertThrows(NullPointerException.class, () ->
service.isAlreadyCached(null));
+ assertThrows(NullPointerException.class, () -> service.getBlockSize(null));
+ assertThrows(NullPointerException.class, () ->
service.onConfigurationChange(null));
+ }
+}
