From e3f9f52c5430130fcf0de5749fcbfbcaadce41de Mon Sep 17 00:00:00 2001 From: Vladimir Rodionov Date: Mon, 8 Jun 2026 21:15:57 -0700 Subject: [PATCH] HBASE-30023 Refactor write path, prefetch, and compaction cache population to use CacheAccessService --- .../hadoop/hbase/io/HalfStoreFileReader.java | 2 +- .../hadoop/hbase/io/hfile/CacheConfig.java | 4 +- .../hbase/io/hfile/HFileBlockIndex.java | 4 +- .../hbase/io/hfile/HFilePreadReader.java | 6 +- .../hbase/io/hfile/HFileReaderImpl.java | 8 +- .../hbase/io/hfile/HFileWriterImpl.java | 5 +- .../BlockCacheBackedCacheAccessService.java | 16 ++++ .../io/hfile/cache/CacheAccessService.java | 86 +++++++++++++++++-- .../io/hfile/cache/CacheRequestContext.java | 12 +-- .../io/hfile/cache/CacheWriteContext.java | 10 +-- .../io/hfile/cache/CacheWriteSource.java | 10 +++ .../hfile/cache/NoOpCacheAccessService.java | 16 ++++ .../hbase/client/FromClientSideTest5.java | 4 +- .../hadoop/hbase/io/hfile/TestHFile.java | 1 + .../hadoop/hbase/io/hfile/TestHFileBlock.java | 4 +- .../hbase/io/hfile/TestHFileBlockIndex.java | 9 +- .../hfile/TestLazyDataBlockDecompression.java | 4 + ...estBlockCacheBackedCacheAccessService.java | 12 +-- .../cache/TestNoOpCacheAccessService.java | 8 +- .../TestTopologyBackedCacheAccessService.java | 8 +- .../regionserver/TestAtomicOperation.java | 4 +- .../TestCacheOnWriteInSchema.java | 4 +- 22 files changed, 179 insertions(+), 58 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java index 9d99904e2131..c8a9708e1d1d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java @@ -365,7 +365,7 @@ public void close(boolean evictOnClose) throws IOException { long offset = s.getCurBlock().getOffset(); LOG.trace("Seeking to split cell in reader: {} for file: {} top: {}, split offset: {}", this, reference, top, offset); - ((HFileReaderImpl) reader).getCacheConf().getBlockCache().ifPresent(cache -> { + ((HFileReaderImpl) reader).getCacheConf().getCacheAccessService().ifEnabled(cache -> { int numEvictedReferred = top ? cache.evictBlocksRangeByHfileName(referred, offset, Long.MAX_VALUE) : cache.evictBlocksRangeByHfileName(referred, 0, offset); 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 3505f8228140..840ec45fbab4 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 @@ -309,8 +309,8 @@ public boolean shouldCacheBlockOnRead(BlockCategory category, HFileInfo hFileInf Configuration conf) { Optional cacheFileBlock = Optional.of(true); // For DATA blocks only, if BucketCache is in use, we don't need to cache block again - if (getBlockCache().isPresent() && category.equals(BlockCategory.DATA)) { - Optional result = getBlockCache().get().shouldCacheFile(hFileInfo, conf); + if (category.equals(BlockCategory.DATA)) { + Optional result = getCacheAccessService().shouldCacheFile(hFileInfo, conf); if (result.isPresent()) { cacheFileBlock = result; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java index 89460900e21d..3066b2702259 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java @@ -1075,7 +1075,7 @@ public long writeIndexBlocks(FSDataOutputStream out) throws IOException { if (midKeyMetadata != null) blockStream.write(midKeyMetadata); blockWriter.writeHeaderAndData(out); if (cacheConf != null) { - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { HFileBlock blockForCaching = blockWriter.getBlockForCaching(cacheConf); cache.cacheBlock(new BlockCacheKey(nameForCaching, rootLevelIndexPos, true, blockForCaching.getBlockType()), blockForCaching); @@ -1169,7 +1169,7 @@ private void writeIntermediateBlock(FSDataOutputStream out, BlockIndexChunk pare blockWriter.writeHeaderAndData(out); if (getCacheOnWrite()) { - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { HFileBlock blockForCaching = blockWriter.getBlockForCaching(cacheConf); try { cache.cacheBlock( 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 2d4a01745170..815f7a8b7f55 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 @@ -42,7 +42,7 @@ public HFilePreadReader(ReaderContext context, HFileInfo fileInfo, CacheConfig c fileInfo.initMetaAndIndex(this); // master hosted regions, like the master procedures store wouldn't have a block cache // Prefetch file blocks upon open if requested - if (cacheConf.getBlockCache().isPresent() && cacheConf.shouldPrefetchOnOpen()) { + if (cacheConf.getCacheAccessService().isCacheEnabled() && cacheConf.shouldPrefetchOnOpen()) { PrefetchExecutor.request(path, new Runnable() { @Override public void run() { @@ -50,7 +50,7 @@ public void run() { long end = 0; HFile.Reader prefetchStreamReader = null; try { - cacheConf.getBlockCache().ifPresent( + cacheConf.getCacheAccessService().ifEnabled( cache -> cache.waitForCacheInitialization(WAIT_TIME_FOR_CACHE_INITIALIZATION)); ReaderContext streamReaderContext = ReaderContextBuilder.newBuilder(context) .withReaderType(ReaderContext.ReaderType.STREAM) @@ -185,7 +185,7 @@ public void close(boolean evictOnClose) throws IOException { // Deallocate blocks in load-on-open section this.fileInfo.close(); // Deallocate data blocks - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { if (evictOnClose) { int numEvicted = cache.evictBlocksByHfileName(name); if (LOG.isTraceEnabled()) { 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 6e09e0ba345e..931afc145956 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 @@ -1271,7 +1271,7 @@ public HFileBlock getMetaBlock(String metaBlockName, boolean cacheBlock) throws // Cache the block if (cacheBlock) { - cacheConf.getBlockCache().ifPresent( + cacheConf.getCacheAccessService().ifEnabled( cache -> cache.cacheBlock(cacheKey, uncompressedBlock, cacheConf.isInMemory())); } return uncompressedBlock; @@ -1290,7 +1290,7 @@ public HFileBlock getMetaBlock(String metaBlockName, boolean cacheBlock) throws * boolean, boolean) */ private boolean shouldUseHeap(BlockType expectedBlockType, boolean cacheBlock) { - if (!cacheConf.getBlockCache().isPresent()) { + if (!cacheConf.getCacheAccessService().isCacheEnabled()) { return false; } @@ -1411,7 +1411,7 @@ public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize, final bo // Don't need the unpacked block back and we're storing the block in the cache compressed if (cacheOnly && cacheCompressed && cacheOnRead) { HFileBlock blockNoChecksum = BlockCacheUtil.getBlockForCaching(cacheConf, hfileBlock); - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { LOG.debug("Skipping decompression of block {} in prefetch", cacheKey); // Cache the block if necessary if (cacheBlock && cacheOnRead) { @@ -1427,7 +1427,7 @@ public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize, final bo HFileBlock unpacked = hfileBlock.unpack(hfileContext, fsBlockReader); HFileBlock unpackedNoChecksum = BlockCacheUtil.getBlockForCaching(cacheConf, unpacked); // Cache the block if necessary - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { if (cacheBlock && cacheOnRead) { // Using the wait on cache during compaction and prefetching. cache.cacheBlock(cacheKey, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java index b635c2cfec63..91f1326552f1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java @@ -52,6 +52,7 @@ import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding; import org.apache.hadoop.hbase.io.encoding.IndexBlockEncoding; import org.apache.hadoop.hbase.io.hfile.HFileBlock.BlockWritable; +import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService; import org.apache.hadoop.hbase.regionserver.TimeRangeTracker; import org.apache.hadoop.hbase.security.EncryptionUtil; import org.apache.hadoop.hbase.security.User; @@ -576,7 +577,7 @@ private void writeInlineBlocks(boolean closing) throws IOException { * @param offset the offset of the block we want to cache. Used to determine the cache key. */ private void doCacheOnWrite(long offset) { - cacheConf.getBlockCache().ifPresent(cache -> { + cacheConf.getCacheAccessService().ifEnabled(cache -> { HFileBlock cacheFormatBlock = blockWriter.getBlockForCaching(cacheConf); try { BlockCacheKey key = buildCacheBlockKey(offset, cacheFormatBlock.getBlockType()); @@ -598,7 +599,7 @@ private BlockCacheKey buildCacheBlockKey(long offset, BlockType blockType) { return new BlockCacheKey(name, offset, true, blockType); } - private boolean shouldCacheBlock(BlockCache cache, BlockCacheKey key) { + private boolean shouldCacheBlock(CacheAccessService cache, BlockCacheKey key) { Optional result = cache.shouldCacheBlock(key, timeRangeTrackerForTiering.get().getMax(), conf); return result.orElse(true); 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 32c012c96f8c..91441f5deb03 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 @@ -27,6 +27,7 @@ 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.io.hfile.HFileInfo; import org.apache.yetus.audience.InterfaceAudience; /** @@ -305,4 +306,19 @@ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int d Objects.requireNonNull(fileName, "fileName must not be null"); blockCache.notifyFileCachingCompleted(fileName, totalBlockCount, dataBlockCount, size); } + + @Override + public Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { + Objects.requireNonNull(hFileInfo, "hFileInfo must not be null"); + Objects.requireNonNull(conf, "conf must not be null"); + return blockCache.shouldCacheFile(hFileInfo, conf); + } + + @Override + public Optional shouldCacheBlock(BlockCacheKey key, long maxTimestamp, + Configuration conf) { + Objects.requireNonNull(key, "key must not be null"); + Objects.requireNonNull(conf, "conf must not be null"); + return blockCache.shouldCacheBlock(key, maxTimestamp, conf); + } } 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 b4660a71dbf1..ce565518b982 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 @@ -17,7 +17,9 @@ */ package org.apache.hadoop.hbase.io.hfile.cache; +import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.io.hfile.BlockCacheKey; @@ -25,6 +27,7 @@ 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.io.hfile.HFileInfo; import org.apache.yetus.audience.InterfaceAudience; /** @@ -119,8 +122,8 @@ public interface CacheAccessService { */ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repeat, boolean updateCacheMetrics) { - CacheRequestContext context = CacheRequestContext.newBuilder().setCaching(caching) - .setRepeat(repeat).setUpdateCacheMetrics(updateCacheMetrics).build(); + CacheRequestContext context = CacheRequestContext.newBuilder().withCaching(caching) + .withRepeat(repeat).withUpdateCacheMetrics(updateCacheMetrics).build(); return getBlock(cacheKey, context); } @@ -140,8 +143,9 @@ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repe */ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repeat, boolean updateCacheMetrics, BlockType blockType) { - CacheRequestContext context = CacheRequestContext.newBuilder().setCaching(caching) - .setRepeat(repeat).setUpdateCacheMetrics(updateCacheMetrics).setBlockType(blockType).build(); + CacheRequestContext context = + CacheRequestContext.newBuilder().withCaching(caching).withRepeat(repeat) + .withUpdateCacheMetrics(updateCacheMetrics).withBlockType(blockType).build(); return getBlock(cacheKey, context); } @@ -178,7 +182,7 @@ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repe * @param inMemory whether the block should be treated as in-memory */ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemory) { - CacheWriteContext context = CacheWriteContext.newBuilder().setInMemory(inMemory).build(); + CacheWriteContext context = CacheWriteContext.newBuilder().withInMemory(inMemory).build(); cacheBlock(cacheKey, block, context); } @@ -196,8 +200,8 @@ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemor */ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemory, boolean waitWhenCache) { - CacheWriteContext context = - CacheWriteContext.newBuilder().setInMemory(inMemory).setWaitWhenCache(waitWhenCache).build(); + CacheWriteContext context = CacheWriteContext.newBuilder().withInMemory(inMemory) + .withWaitWhenCache(waitWhenCache).build(); cacheBlock(cacheKey, block, context); } @@ -444,4 +448,72 @@ default void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int long size) { // noop } + + /** + * Executes the supplied action when this cache service is enabled. + *

+ * This helper is intended to preserve the old {@code getBlockCache().ifPresent(...)} style for + * call sites that should do nothing when block cache is disabled. It keeps callers independent of + * concrete implementations such as {@code NoOpCacheAccessService} while still allowing disabled + * cache wiring to behave like an absent cache. + *

+ *

+ * Implementations normally do not need to override this method. The default implementation checks + * {@link #isCacheEnabled()} and invokes the supplied action only when cache access is enabled. + *

+ * @param action action to execute with this cache service when enabled + */ + default void ifEnabled(Consumer action) { + Objects.requireNonNull(action, "action must not be null"); + if (isCacheEnabled()) { + action.accept(this); + } + } + + /** + * Returns whether blocks from the given HFile should be cached. TODO: this method is a temporary + * adapter for file-level admission decisions. It will be removed and replaced by a more general + * admission API in the future. + *

+ * This is a file-level admission hook used by cache population paths. Implementations may use + * file metadata, configuration, data tiering state, or implementation-specific bookkeeping to + * decide whether the file should be admitted into cache. + *

+ *

+ * The returned {@link Optional} is empty when the cache service does not support file-level + * admission decisions. In that case, callers should preserve existing default behavior, typically + * treating the result as "no opinion" rather than as a rejection. + *

+ * @param hFileInfo HFile metadata used for the admission decision + * @param conf configuration + * @return empty if unsupported; otherwise whether the file should be cached + */ + default Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { + return Optional.empty(); + } + + /** + * Returns whether the block represented by the given key and timestamp should be cached. TODO: + * this method is a temporary adapter for file-level admission decisions. It will be removed and + * replaced by a more general admission API in the future. + *

+ *

+ * This is a block-level admission hook used by cache population paths. Implementations may use + * block identity, maximum timestamp, configuration, data tiering state, or + * implementation-specific policy to decide whether the block should be admitted into cache. + *

+ *

+ * The returned {@link Optional} is empty when the cache service does not support block-level + * admission decisions. In that case, callers should preserve existing default behavior, typically + * treating the result as "no opinion" rather than as a rejection. + *

+ * @param key block cache key + * @param maxTimestamp maximum timestamp associated with the block + * @param conf configuration + * @return empty if unsupported; otherwise whether the block should be cached + */ + default Optional shouldCacheBlock(BlockCacheKey key, long maxTimestamp, + Configuration conf) { + return Optional.empty(); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheRequestContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheRequestContext.java index 4b04d9bf11b5..fd9cd51ff81e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheRequestContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheRequestContext.java @@ -122,7 +122,7 @@ private Builder() { * @param caching true when caching is enabled * @return this builder */ - public Builder setCaching(boolean caching) { + public Builder withCaching(boolean caching) { this.caching = caching; return this; } @@ -132,7 +132,7 @@ public Builder setCaching(boolean caching) { * @param repeat true when this is a repeated lookup * @return this builder */ - public Builder setRepeat(boolean repeat) { + public Builder withRepeat(boolean repeat) { this.repeat = repeat; return this; } @@ -142,7 +142,7 @@ public Builder setRepeat(boolean repeat) { * @param updateCacheMetrics true when metrics should be updated * @return this builder */ - public Builder setUpdateCacheMetrics(boolean updateCacheMetrics) { + public Builder withUpdateCacheMetrics(boolean updateCacheMetrics) { this.updateCacheMetrics = updateCacheMetrics; return this; } @@ -152,7 +152,7 @@ public Builder setUpdateCacheMetrics(boolean updateCacheMetrics) { * @param blockType expected block type * @return this builder */ - public Builder setBlockType(BlockType blockType) { + public Builder withBlockType(BlockType blockType) { this.blockType = blockType; return this; } @@ -162,7 +162,7 @@ public Builder setBlockType(BlockType blockType) { * @param compaction true when compaction-related * @return this builder */ - public Builder setCompaction(boolean compaction) { + public Builder withCompaction(boolean compaction) { this.compaction = compaction; return this; } @@ -172,7 +172,7 @@ public Builder setCompaction(boolean compaction) { * @param prefetch true when prefetch-related * @return this builder */ - public Builder setPrefetch(boolean prefetch) { + public Builder withPrefetch(boolean prefetch) { this.prefetch = prefetch; return this; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteContext.java index e9b2f729f5ba..848d12ce44e7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteContext.java @@ -110,7 +110,7 @@ private Builder() { * @param inMemory true when in-memory treatment is requested * @return this builder */ - public Builder setInMemory(boolean inMemory) { + public Builder withInMemory(boolean inMemory) { this.inMemory = inMemory; return this; } @@ -120,7 +120,7 @@ public Builder setInMemory(boolean inMemory) { * @param waitWhenCache true when insertion should wait * @return this builder */ - public Builder setWaitWhenCache(boolean waitWhenCache) { + public Builder withWaitWhenCache(boolean waitWhenCache) { this.waitWhenCache = waitWhenCache; return this; } @@ -130,7 +130,7 @@ public Builder setWaitWhenCache(boolean waitWhenCache) { * @param cacheCompressed true when compressed caching is requested * @return this builder */ - public Builder setCacheCompressed(boolean cacheCompressed) { + public Builder withCacheCompressed(boolean cacheCompressed) { this.cacheCompressed = cacheCompressed; return this; } @@ -140,7 +140,7 @@ public Builder setCacheCompressed(boolean cacheCompressed) { * @param blockCategory block category * @return this builder */ - public Builder setBlockCategory(BlockCategory blockCategory) { + public Builder withBlockCategory(BlockCategory blockCategory) { this.blockCategory = blockCategory; return this; } @@ -150,7 +150,7 @@ public Builder setBlockCategory(BlockCategory blockCategory) { * @param source cache write source * @return this builder */ - public Builder setSource(CacheWriteSource source) { + public Builder withSource(CacheWriteSource source) { this.source = source; return this; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteSource.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteSource.java index 2c98d9d7c941..987587caafcc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteSource.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheWriteSource.java @@ -35,6 +35,16 @@ public enum CacheWriteSource { */ READ_MISS, + /** + * Block was inserted from the HFile write/cache-on-write path. + *

+ * This source covers write-side population where the caller does not expose whether the HFile is + * being produced by flush, compaction, bulk load, or another writer. More specific write-side + * sources can be added later when that context is available at the call site. + *

+ */ + WRITE_PATH, + /** * Cache population during flush output generation. */ 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 5c9da57aa10c..938541e93eea 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 @@ -25,6 +25,7 @@ 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.io.hfile.HFileInfo; import org.apache.yetus.audience.InterfaceAudience; /** @@ -290,4 +291,19 @@ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int d long size) { Objects.requireNonNull(fileName, "fileName must not be null"); } + + @Override + public Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { + Objects.requireNonNull(hFileInfo, "hFileInfo must not be null"); + Objects.requireNonNull(conf, "conf must not be null"); + return Optional.empty(); + } + + @Override + public Optional shouldCacheBlock(BlockCacheKey key, long maxTimestamp, + Configuration conf) { + Objects.requireNonNull(key, "key must not be null"); + Objects.requireNonNull(conf, "conf must not be null"); + return Optional.empty(); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/FromClientSideTest5.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/FromClientSideTest5.java index 2bd8bce55ca3..5c924b7de8e4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/FromClientSideTest5.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/FromClientSideTest5.java @@ -64,8 +64,8 @@ import org.apache.hadoop.hbase.filter.RowFilter; import org.apache.hadoop.hbase.filter.SubstringComparator; import org.apache.hadoop.hbase.filter.ValueFilter; -import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.CacheConfig; +import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.HStore; @@ -650,7 +650,7 @@ public void testCacheOnWriteEvictOnClose() throws Exception { CacheConfig cacheConf = store.getCacheConfig(); cacheConf.setCacheDataOnWrite(true); cacheConf.setEvictOnClose(true); - BlockCache cache = cacheConf.getBlockCache().get(); + CacheAccessService cache = cacheConf.getCacheAccessService(); // establish baseline stats long startBlockCount = cache.getBlockCount(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java index 3486cd0d1eaa..5d470a60a3da 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java @@ -219,6 +219,7 @@ public void onLeak(String s, String s1) { BlockCache cache = Mockito.mock(BlockCache.class); Mockito.when(cache.shouldCacheBlock(Mockito.any(), Mockito.anyLong(), Mockito.any())) .thenReturn(Optional.of(false)); + Mockito.when(cache.isCacheEnabled()).thenReturn(true); Path hfilePath = new Path(TEST_UTIL.getDataTestDir(), "testWriterCacheOnWriteSkipDoesNotLeak"); HFileContext context = new HFileContextBuilder().withBlockSize(blockSize).build(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java index 10b92e17239c..d8ad4aa1db33 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java @@ -36,7 +36,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Random; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; @@ -65,6 +64,7 @@ import org.apache.hadoop.hbase.io.compress.Compression; import org.apache.hadoop.hbase.io.compress.Compression.Algorithm; import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding; +import org.apache.hadoop.hbase.io.hfile.cache.NoOpCacheAccessService; import org.apache.hadoop.hbase.nio.ByteBuff; import org.apache.hadoop.hbase.nio.MultiByteBuff; import org.apache.hadoop.hbase.nio.SingleByteBuff; @@ -288,7 +288,7 @@ public String createTestBlockStr(Compression.Algorithm algo, int correctLength, @TestTemplate public void testNoCompression() throws IOException { CacheConfig cacheConf = Mockito.mock(CacheConfig.class); - Mockito.when(cacheConf.getBlockCache()).thenReturn(Optional.empty()); + Mockito.when(cacheConf.getCacheAccessService()).thenReturn(new NoOpCacheAccessService()); HFileBlock block = createTestV2Block(NONE, includesMemstoreTS, false).getBlockForCaching(cacheConf); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java index 1d7920f438b4..26e118e284c7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java @@ -60,6 +60,7 @@ import org.apache.hadoop.hbase.io.hfile.HFile.Writer; import org.apache.hadoop.hbase.io.hfile.HFileBlockIndex.BlockIndexReader; import org.apache.hadoop.hbase.io.hfile.NoOpIndexBlockEncoder.NoOpEncodedSeeker; +import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService; import org.apache.hadoop.hbase.nio.ByteBuff; import org.apache.hadoop.hbase.nio.MultiByteBuff; import org.apache.hadoop.hbase.nio.RefCnt; @@ -619,9 +620,9 @@ public void testMidKeyOnLeafIndexBlockBoundary() throws IOException { // should open hfile.block.index.cacheonwrite conf.setBoolean(CacheConfig.CACHE_INDEX_BLOCKS_ON_WRITE_KEY, true); CacheConfig cacheConf = new CacheConfig(conf, BlockCacheFactory.createBlockCache(conf)); - BlockCache blockCache = cacheConf.getBlockCache().get(); + CacheAccessService cache = cacheConf.getCacheAccessService(); // Evict all blocks that were cached-on-write by the previous invocation. - blockCache.evictBlocksByHfileName(hfilePath.getName()); + cache.evictBlocksByHfileName(hfilePath.getName()); // Write the HFile HFileContext meta = new HFileContextBuilder().withBlockSize(SMALL_BLOCK_SIZE) .withCompression(Algorithm.NONE).withDataBlockEncoding(DataBlockEncoding.NONE).build(); @@ -671,14 +672,14 @@ public void testMidKeyOnLeafIndexBlockBoundary() throws IOException { public void testHFileWriterAndReader() throws IOException { Path hfilePath = new Path(TEST_UTIL.getDataTestDir(), "hfile_for_block_index"); CacheConfig cacheConf = new CacheConfig(conf, BlockCacheFactory.createBlockCache(conf)); - BlockCache blockCache = cacheConf.getBlockCache().get(); + CacheAccessService cache = cacheConf.getCacheAccessService(); for (int testI = 0; testI < INDEX_CHUNK_SIZES.length; ++testI) { int indexBlockSize = INDEX_CHUNK_SIZES[testI]; int expectedNumLevels = EXPECTED_NUM_LEVELS[testI]; LOG.info("Index block size: " + indexBlockSize + ", compression: " + compr); // Evict all blocks that were cached-on-write by the previous invocation. - blockCache.evictBlocksByHfileName(hfilePath.getName()); + cache.evictBlocksByHfileName(hfilePath.getName()); conf.setInt(HFileBlockIndex.MAX_CHUNK_SIZE_KEY, indexBlockSize); Set keyStrSet = new HashSet<>(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLazyDataBlockDecompression.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLazyDataBlockDecompression.java index f3fe26b2baa0..a81ecd530e05 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLazyDataBlockDecompression.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLazyDataBlockDecompression.java @@ -132,6 +132,10 @@ private static void cacheBlocks(Configuration conf, CacheConfig cacheConfig, Fil reader.close(); } + /* + * TODO: migrate this test to use new HBASE-30018 APIs and remove the need to cast to + * LruBlockCache + */ @TestTemplate public void testCompressionIncreasesEffectiveBlockCacheSize() throws Exception { // enough room for 2 uncompressed block 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 55dd5a579dbb..8a7378c7b837 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 @@ -69,8 +69,8 @@ void testGetBlockWithBlockTypeDelegatesToBlockCache() { when(blockCache.getBlock(key, true, true, false, BlockType.DATA)).thenReturn(block); - CacheRequestContext context = CacheRequestContext.newBuilder().setCaching(true).setRepeat(true) - .setUpdateCacheMetrics(false).setBlockType(BlockType.DATA).build(); + CacheRequestContext context = CacheRequestContext.newBuilder().withCaching(true) + .withRepeat(true).withUpdateCacheMetrics(false).withBlockType(BlockType.DATA).build(); assertSame(block, service.getBlock(key, context)); verify(blockCache).getBlock(key, true, true, false, BlockType.DATA); @@ -88,8 +88,8 @@ void testGetBlockWithoutBlockTypeDelegatesToBlockCache() { when(blockCache.getBlock(key, true, false, true)).thenReturn(block); - CacheRequestContext context = CacheRequestContext.newBuilder().setCaching(true).setRepeat(false) - .setUpdateCacheMetrics(true).build(); + CacheRequestContext context = CacheRequestContext.newBuilder().withCaching(true) + .withRepeat(false).withUpdateCacheMetrics(true).build(); assertSame(block, service.getBlock(key, context)); verify(blockCache).getBlock(key, true, false, true); @@ -105,8 +105,8 @@ void testCacheBlockDelegatesToBlockCache() { BlockCacheKey key = new BlockCacheKey(HFILE_NAME, BLOCK_OFFSET); Cacheable block = mock(Cacheable.class); - CacheWriteContext context = CacheWriteContext.newBuilder().setInMemory(true) - .setWaitWhenCache(true).setSource(CacheWriteSource.READ_MISS).build(); + CacheWriteContext context = CacheWriteContext.newBuilder().withInMemory(true) + .withWaitWhenCache(true).withSource(CacheWriteSource.READ_MISS).build(); service.cacheBlock(key, block, context); 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 index 582aa7e02410..e73da8cd9414 100644 --- 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 @@ -52,10 +52,10 @@ public class TestNoOpCacheAccessService { 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(); + CacheRequestContext requestContext = CacheRequestContext.newBuilder().withCaching(true) + .withRepeat(false).withUpdateCacheMetrics(true).build(); + CacheWriteContext writeContext = CacheWriteContext.newBuilder().withInMemory(true) + .withWaitWhenCache(true).withSource(CacheWriteSource.READ_MISS).build(); Cacheable block = mock(Cacheable.class); Configuration conf = new Configuration(false); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessService.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessService.java index 7d37a4dd110d..e9001553345a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessService.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessService.java @@ -463,12 +463,12 @@ void testFactoryCreatesTopologyBackedService() { } private static CacheRequestContext requestContext() { - return CacheRequestContext.newBuilder().setCaching(true).setRepeat(false) - .setUpdateCacheMetrics(true).setBlockType(BlockType.DATA).build(); + return CacheRequestContext.newBuilder().withCaching(true).withRepeat(false) + .withUpdateCacheMetrics(true).withBlockType(BlockType.DATA).build(); } private static CacheWriteContext writeContext() { - return CacheWriteContext.newBuilder().setInMemory(true).setWaitWhenCache(true) - .setSource(CacheWriteSource.READ_MISS).build(); + return CacheWriteContext.newBuilder().withInMemory(true).withWaitWhenCache(true) + .withSource(CacheWriteSource.READ_MISS).build(); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java index 057b12ed64a7..28b6e03b11e2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java @@ -63,8 +63,8 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.filter.BinaryComparator; import org.apache.hadoop.hbase.io.HeapSize; -import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.CacheConfig; +import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.VerySlowRegionServerTests; import org.apache.hadoop.hbase.util.Bytes; @@ -115,7 +115,7 @@ public void teardown() throws IOException { if (wal != null) { wal.close(); } - cacheConfig.getBlockCache().ifPresent(BlockCache::shutdown); + cacheConfig.getCacheAccessService().ifEnabled(CacheAccessService::shutdown); region = null; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCacheOnWriteInSchema.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCacheOnWriteInSchema.java index b7e42659f26e..b13b619aea15 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCacheOnWriteInSchema.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCacheOnWriteInSchema.java @@ -37,7 +37,6 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.fs.HFileSystem; import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding; -import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.BlockCacheFactory; import org.apache.hadoop.hbase.io.hfile.BlockCacheKey; import org.apache.hadoop.hbase.io.hfile.BlockType; @@ -46,6 +45,7 @@ import org.apache.hadoop.hbase.io.hfile.HFileBlock; import org.apache.hadoop.hbase.io.hfile.HFileScanner; import org.apache.hadoop.hbase.io.hfile.RandomKeyValueUtil; +import org.apache.hadoop.hbase.io.hfile.cache.CacheAccessService; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Bytes; @@ -201,7 +201,7 @@ public void testCacheOnWriteInSchema() throws IOException { private void readStoreFile(Path path) throws IOException { CacheConfig cacheConf = store.getCacheConfig(); - BlockCache cache = cacheConf.getBlockCache().get(); + CacheAccessService cache = cacheConf.getCacheAccessService(); StoreFileInfo storeFileInfo = StoreFileInfo.createStoreFileInfoForHFile(conf, fs, path, true); HStoreFile sf = new HStoreFile(storeFileInfo, BloomType.ROWCOL, cacheConf); sf.initReader();