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 a90bc700b5df..fa69b77f5ecb 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 @@ -27,6 +27,8 @@ 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.CacheAccessServices; +import org.apache.hadoop.hbase.io.hfile.cache.CacheTopologyType; +import org.apache.hadoop.hbase.io.hfile.cache.TopologyBackedCacheAccessService; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -527,7 +529,20 @@ public CacheAccessService getCacheAccessService() { } public boolean isCombinedBlockCache() { - return blockCache instanceof CombinedBlockCache; + if (blockCache instanceof CombinedBlockCache) { + return true; + } + return isCombinedBlockCacheCompatible(cacheAccessService); + } + + private static boolean isCombinedBlockCacheCompatible(CacheAccessService cacheAccessService) { + if (!(cacheAccessService instanceof TopologyBackedCacheAccessService)) { + return false; + } + + TopologyBackedCacheAccessService service = + (TopologyBackedCacheAccessService) cacheAccessService; + return service.getTopology().getType() == CacheTopologyType.TIERED_EXCLUSIVE; } public ByteBuffAllocator getByteBuffAllocator() { 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 e55b03f6fd87..aa8f447a055f 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.io.hfile.cache; import java.util.Iterator; +import java.util.Map; import java.util.Objects; import java.util.Optional; import org.apache.hadoop.conf.Configuration; @@ -30,6 +31,7 @@ import org.apache.hadoop.hbase.io.hfile.CachedBlock; import org.apache.hadoop.hbase.io.hfile.HFileBlock; import org.apache.hadoop.hbase.io.hfile.HFileInfo; +import org.apache.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; /** @@ -310,6 +312,11 @@ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int d blockCache.notifyFileCachingCompleted(fileName, totalBlockCount, dataBlockCount, size); } + @Override + public Optional>> getFullyCachedFiles() { + return blockCache.getFullyCachedFiles(); + } + @Override public Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { Objects.requireNonNull(hFileInfo, "hFileInfo must not be null"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheEngine.java index 98fca26b46e2..ae87801ede9e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheEngine.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/BlockCacheBackedCacheEngine.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.io.hfile.cache; +import java.util.Map; import java.util.Objects; import java.util.Optional; import org.apache.hadoop.conf.Configuration; @@ -26,7 +27,10 @@ import org.apache.hadoop.hbase.io.hfile.BlockType; import org.apache.hadoop.hbase.io.hfile.CacheStats; import org.apache.hadoop.hbase.io.hfile.Cacheable; +import org.apache.hadoop.hbase.io.hfile.FirstLevelBlockCache; import org.apache.hadoop.hbase.io.hfile.HFileBlock; +import org.apache.hadoop.hbase.io.hfile.HFileInfo; +import org.apache.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; /** @@ -184,9 +188,13 @@ public Optional blockFitsIntoTheCache(HFileBlock block) { } @Override - public Optional isAlreadyCached(BlockCacheKey key) { - Objects.requireNonNull(key, "key must not be null"); - return blockCache.isAlreadyCached(key); + public Optional isAlreadyCached(BlockCacheKey cacheKey) { + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + if (blockCache instanceof FirstLevelBlockCache) { + FirstLevelBlockCache firstLevelBlockCache = (FirstLevelBlockCache) blockCache; + return Optional.of(firstLevelBlockCache.containsBlock(cacheKey)); + } + return blockCache.isAlreadyCached(cacheKey); } @Override @@ -217,4 +225,24 @@ 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); + } + + @Override + public Optional>> getFullyCachedFiles() { + return blockCache.getFullyCachedFiles(); + } } 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 d5dfa54ea279..ceadedcd6ddf 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,6 +17,7 @@ */ package org.apache.hadoop.hbase.io.hfile.cache; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; @@ -29,6 +30,7 @@ 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.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; /** @@ -123,6 +125,7 @@ public interface CacheAccessService extends ConfigurationObserver { */ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repeat, boolean updateCacheMetrics) { + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); CacheRequestContext context = CacheRequestContext.newBuilder().withCaching(caching) .withRepeat(repeat).withUpdateCacheMetrics(updateCacheMetrics).build(); return getBlock(cacheKey, context); @@ -144,6 +147,7 @@ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repe */ default Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean repeat, boolean updateCacheMetrics, BlockType blockType) { + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); CacheRequestContext context = CacheRequestContext.newBuilder().withCaching(caching).withRepeat(repeat) .withUpdateCacheMetrics(updateCacheMetrics).withBlockType(blockType).build(); @@ -183,7 +187,10 @@ 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().withInMemory(inMemory).build(); + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + Objects.requireNonNull(block, "block must not be null"); + CacheWriteContext context = CacheWriteContext.newBuilder().withInMemory(inMemory) + .withBlockCategory(block.getBlockType().getCategory()).build(); cacheBlock(cacheKey, block, context); } @@ -199,10 +206,14 @@ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemor * @param inMemory whether the block should be treated as in-memory * @param waitWhenCache whether to wait for the cache operation to be accepted/flushed */ + default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemory, boolean waitWhenCache) { - CacheWriteContext context = CacheWriteContext.newBuilder().withInMemory(inMemory) - .withWaitWhenCache(waitWhenCache).build(); + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + Objects.requireNonNull(block, "block must not be null"); + CacheWriteContext context = + CacheWriteContext.newBuilder().withInMemory(inMemory).withWaitWhenCache(waitWhenCache) + .withBlockCategory(block.getBlockType().getCategory()).build(); cacheBlock(cacheKey, block, context); } @@ -215,7 +226,11 @@ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block, boolean inMemor * @param block block contents */ default void cacheBlock(BlockCacheKey cacheKey, Cacheable block) { - cacheBlock(cacheKey, block, CacheWriteContext.newBuilder().build()); + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + Objects.requireNonNull(block, "block must not be null"); + CacheWriteContext context = + CacheWriteContext.newBuilder().withBlockCategory(block.getBlockType().getCategory()).build(); + cacheBlock(cacheKey, block, context); } /** @@ -524,4 +539,19 @@ default Optional shouldCacheBlock(BlockCacheKey key, long maxTimestamp, Configuration conf) { return Optional.empty(); } + + /** + * Returns the files that are fully cached by this cache implementation. + *

+ * A file is considered fully cached when all of its cacheable blocks are present in the cache. + * Not all cache implementations track this information. Implementations that do not support this + * capability should return {@link Optional#empty()}. + *

+ * @return an {@link Optional} containing a map of fully cached files when this capability is + * supported; otherwise {@link Optional#empty()} + */ + default Optional>> getFullyCachedFiles() { + return Optional.empty(); + } + } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java index d4e739fcdcce..6e9e34a5c695 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java @@ -23,6 +23,7 @@ import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.BlockCacheFactory; import org.apache.hadoop.hbase.io.hfile.CachedBlock; +import org.apache.hadoop.hbase.io.hfile.CombinedBlockCache; import org.apache.yetus.audience.InterfaceAudience; /** @@ -46,18 +47,26 @@ private CacheAccessServices() { } /** - * Creates a {@link CacheAccessService} backed by an existing {@link BlockCache}. + * Creates a cache access service backed by an existing block cache. *

- * This is the default compatibility path during migration from {@code BlockCache} to - * {@code CacheAccessService}. The returned service delegates to the supplied block cache and - * should preserve existing behavior. + * For regular {@link BlockCache} implementations, this returns a legacy + * {@link BlockCacheBackedCacheAccessService}. For {@link CombinedBlockCache}, this returns a + * topology-backed service using {@link TieredExclusiveTopology}. This moves combined L1/L2 + * orchestration to the new topology layer while keeping the existing combined block cache object + * available for legacy {@link BlockCache}-facing APIs. *

- * @param blockCache block cache to wrap - * @return cache access service backed by {@code blockCache} + * @param blockCache block cache to expose through {@link CacheAccessService} + * @return cache access service */ + public static CacheAccessService fromBlockCache(BlockCache blockCache) { - return new BlockCacheBackedCacheAccessService( - Objects.requireNonNull(blockCache, "blockCache must not be null")); + Objects.requireNonNull(blockCache, "blockCache must not be null"); + if (blockCache instanceof CombinedBlockCache) { + return TopologyBackedCacheAccessServices + .fromCombinedBlockCache((CombinedBlockCache) blockCache); + } + return new BlockCacheBackedCacheAccessService(blockCache); + } /** 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 b5564657dfbb..6df7b2be9f83 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 @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.io.hfile.cache; +import java.util.Map; import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -25,6 +26,8 @@ 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.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; /** @@ -302,4 +305,17 @@ default void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int long size) { // noop } + + default Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { + return Optional.empty(); + } + + default Optional shouldCacheBlock(BlockCacheKey key, long maxTimeStamp, + Configuration conf) { + return Optional.empty(); + } + + default Optional>> getFullyCachedFiles() { + return Optional.empty(); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TierDecision.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TierDecision.java index fb0434b0ff73..d45151c093f1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TierDecision.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TierDecision.java @@ -112,4 +112,9 @@ public List getTiers() { public boolean isEmpty() { return tiers.isEmpty(); } + + @Override + public String toString() { + return "TierDecision{" + "tiers=" + tiers + '}'; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java index b7c278d3ead5..b186ab97ed19 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessService.java @@ -17,16 +17,22 @@ */ package org.apache.hadoop.hbase.io.hfile.cache; +import java.util.HashMap; +import java.util.Map; 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.BlockType; 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.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; + /** * {@link CacheAccessService} implementation backed by {@link CacheTopology} and {@link CacheEngine} * instances. @@ -121,9 +127,59 @@ public String getName() { * @return cached block, or {@code null} if not present in any tier */ @Override + public Cacheable getBlock(BlockCacheKey cacheKey, CacheRequestContext context) { Objects.requireNonNull(cacheKey, "cacheKey must not be null"); Objects.requireNonNull(context, "context must not be null"); + if (topology.getType() == CacheTopologyType.TIERED_EXCLUSIVE) { + return getBlockFromTieredExclusiveTopology(cacheKey, context); + } + return getBlockFromAllTiers(cacheKey, context); + + } + + private Cacheable getBlockFromTieredExclusiveTopology(BlockCacheKey cacheKey, + CacheRequestContext context) { + Optional l1 = topology.getEngine(CacheTier.L1); + Optional l2 = topology.getEngine(CacheTier.L2); + + if (!l1.isPresent() && !l2.isPresent()) { + return null; + } + + if (!l1.isPresent()) { + return getBlockFromEngine(l2.get(), cacheKey, context); + } + + if (!l2.isPresent()) { + return getBlockFromEngine(l1.get(), cacheKey, context); + } + + CacheEngine selectedEngine = l1.get(); + CacheTier selectedTier = CacheTier.L1; + + Optional existsInL1 = l1.get().isAlreadyCached(cacheKey); + if (!existsInL1.orElse(false)) { + selectedEngine = l2.get(); + selectedTier = CacheTier.L2; + } + + Cacheable block = getBlockFromEngine(selectedEngine, cacheKey, context); + boolean updateCacheMetrics = context.isUpdateCacheMetrics(); + boolean caching = context.isCaching(); + if (updateCacheMetrics) { + updateBlockMetrics(block, cacheKey, selectedEngine, caching); + } + + if (block != null) { + maybePromote(cacheKey, block, selectedTier, selectedEngine, context); + } + return block; + } + + private Cacheable getBlockFromAllTiers(BlockCacheKey cacheKey, CacheRequestContext context) { + Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + Objects.requireNonNull(context, "context must not be null"); for (CacheTier tier : topology.getTiers()) { Optional engine = topology.getEngine(tier); @@ -141,6 +197,19 @@ public Cacheable getBlock(BlockCacheKey cacheKey, CacheRequestContext context) { return null; } + private void updateBlockMetrics(Cacheable block, BlockCacheKey key, CacheEngine engine, + boolean caching) { + CacheStats stats = engine.getStats(); + if (stats == null) { + return; + } + if (block == null) { + stats.miss(caching, key.isPrimary(), key.getBlockType()); + } else { + stats.hit(caching, key.isPrimary(), key.getBlockType()); + } + } + /** * Adds a block to the cache using policy-selected target tiers. *

@@ -156,6 +225,7 @@ public Cacheable getBlock(BlockCacheKey cacheKey, CacheRequestContext context) { * @param block block contents * @param context cache write context */ + @Override public void cacheBlock(BlockCacheKey cacheKey, Cacheable block, CacheWriteContext context) { Objects.requireNonNull(cacheKey, "cacheKey must not be null"); @@ -177,15 +247,28 @@ public void cacheBlock(BlockCacheKey cacheKey, Cacheable block, CacheWriteContex } } - /** - * Evicts a single block from all engines participating in the topology. - * @param cacheKey block to remove - * @return {@code true} if at least one engine removed the block, {@code false} otherwise - */ @Override public boolean evictBlock(BlockCacheKey cacheKey) { Objects.requireNonNull(cacheKey, "cacheKey must not be null"); + if (topology.getType() == CacheTopologyType.TIERED_EXCLUSIVE) { + return evictBlockFromFirstMatchingTier(cacheKey); + } + + return evictBlockFromAllTiers(cacheKey); + } + + private boolean evictBlockFromFirstMatchingTier(BlockCacheKey cacheKey) { + for (CacheTier tier : topology.getTiers()) { + Optional engine = topology.getEngine(tier); + if (engine.isPresent() && engine.get().evictBlock(cacheKey)) { + return true; + } + } + return false; + } + + private boolean evictBlockFromAllTiers(BlockCacheKey cacheKey) { boolean evicted = false; for (CacheEngine engine : topology.getEngines()) { evicted |= engine.evictBlock(cacheKey); @@ -456,6 +539,37 @@ public void onConfigurationChange(Configuration config) { } } + @Override + public Optional shouldCacheFile(HFileInfo hFileInfo, Configuration conf) { + Objects.requireNonNull(hFileInfo, "hFileInfo must not be null"); + Objects.requireNonNull(conf, "conf must not be null"); + + boolean shouldCache = true; + for (CacheEngine engine : topology.getEngines()) { + Optional result = engine.shouldCacheFile(hFileInfo, conf); + if (result.isPresent()) { + shouldCache = shouldCache && result.get(); + } + } + return Optional.of(shouldCache); + } + + @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"); + + boolean shouldCache = true; + for (CacheEngine engine : topology.getEngines()) { + Optional result = engine.shouldCacheBlock(key, maxTimeStamp, conf); + if (result.isPresent()) { + shouldCache = shouldCache && result.get(); + } + } + return Optional.of(shouldCache); + } + private Cacheable getBlockFromEngine(CacheEngine engine, BlockCacheKey cacheKey, CacheRequestContext context) { Optional blockType = context.getBlockType(); @@ -483,4 +597,29 @@ private void maybePromote(BlockCacheKey cacheKey, Cacheable block, CacheTier sou topology.promote(cacheKey, block, sourceEngine, targetEngine.get()); } + + @Override + public Optional>> getFullyCachedFiles() { + Map> fullyCachedFiles = new HashMap<>(); + boolean found = false; + + for (CacheEngine engine : topology.getEngines()) { + Optional>> result = engine.getFullyCachedFiles(); + if (result.isPresent()) { + found = true; + fullyCachedFiles.putAll(result.get()); + } + } + + return found ? Optional.of(fullyCachedFiles) : Optional.empty(); + } + + @Override + public void notifyFileCachingCompleted(Path path, int blockCount, int dataBlockCount, long size) { + Objects.requireNonNull(path, "path must not be null"); + + for (CacheEngine engine : topology.getEngines()) { + engine.notifyFileCachingCompleted(path, blockCount, dataBlockCount, size); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java index 60b2b9f6ddfb..3ce586c22a78 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/TopologyBackedCacheAccessServices.java @@ -19,6 +19,8 @@ import java.util.Objects; import org.apache.hadoop.hbase.io.hfile.BlockCache; +import org.apache.hadoop.hbase.io.hfile.CombinedBlockCache; +import org.apache.hadoop.hbase.io.hfile.FirstLevelBlockCache; import org.apache.yetus.audience.InterfaceAudience; /** @@ -30,17 +32,58 @@ * {@link TieredExclusiveTopology}, and exposed through {@link TopologyBackedCacheAccessService}. *

*

- * This class does not change production cache wiring by itself. It only provides a reusable - * construction path for tests and later migration steps that need a CombinedBlockCache-compatible - * topology-backed service. + * This class does not create or remove any concrete cache implementation by itself. It only + * provides a reusable construction path for tests and migration steps that need a + * CombinedBlockCache-compatible topology-backed service. *

*/ @InterfaceAudience.Private public final class TopologyBackedCacheAccessServices { + private static final int COMBINED_BLOCK_CACHE_TIER_COUNT = 2; + private TopologyBackedCacheAccessServices() { } + /** + * Creates a topology-backed cache access service from an existing combined block cache. + *

+ * The supplied {@link CombinedBlockCache} is used only as a legacy holder for the participating + * L1 and L2 {@link BlockCache} instances. The returned service uses + * {@link TieredExclusiveTopology} as the actual orchestration model. + *

+ * @param combinedBlockCache combined block cache containing L1 and L2 caches + * @return topology-backed cache access service + */ + public static TopologyBackedCacheAccessService + fromCombinedBlockCache(CombinedBlockCache combinedBlockCache) { + return fromCombinedBlockCache(combinedBlockCache, + new DefaultHBaseCachePlacementAdmissionPolicy()); + } + + /** + * Creates a topology-backed cache access service from an existing combined block cache. + *

+ * This overload allows tests and future wiring code to provide an explicit policy while still + * extracting L1 and L2 caches from the supplied {@link CombinedBlockCache}. + *

+ * @param combinedBlockCache combined block cache containing L1 and L2 caches + * @param policy placement and admission policy + * @return topology-backed cache access service + */ + public static TopologyBackedCacheAccessService fromCombinedBlockCache( + CombinedBlockCache combinedBlockCache, CachePlacementAdmissionPolicy policy) { + Objects.requireNonNull(combinedBlockCache, "combinedBlockCache must not be null"); + Objects.requireNonNull(policy, "policy must not be null"); + + BlockCache[] blockCaches = combinedBlockCache.getBlockCaches(); + if (blockCaches.length != COMBINED_BLOCK_CACHE_TIER_COUNT) { + throw new IllegalArgumentException("combinedBlockCache must expose exactly two block caches"); + } + + return fromTieredExclusiveBlockCaches("combined", blockCaches[0], blockCaches[1], policy); + } + /** * Creates a topology-backed cache access service from existing L1 and L2 block caches. *

@@ -60,10 +103,31 @@ public static TopologyBackedCacheAccessService fromTieredExclusiveBlockCaches(St Objects.requireNonNull(l1, "l1 must not be null"); Objects.requireNonNull(l2, "l2 must not be null"); Objects.requireNonNull(policy, "policy must not be null"); - + wireVictimCache(l1, l2); CacheEngine l1Engine = CacheEngines.fromBlockCache(l1); CacheEngine l2Engine = CacheEngines.fromBlockCache(l2); CacheTopology topology = new TieredExclusiveTopology(name, l1Engine, l2Engine); return new TopologyBackedCacheAccessService(topology, policy); } + + /** + * Configures the legacy L1 to L2 victim-cache relationship used by CombinedBlockCache. + *

+ * The topology-backed service owns lookup and placement orchestration, but existing + * {@link FirstLevelBlockCache} implementations still use a direct victim-cache reference to move + * evicted blocks from L1 to L2. Keep this wiring while L1 and L2 are still legacy + * {@link BlockCache} implementations. + *

+ * @param l1 first-level block cache + * @param l2 second-level block cache + */ + private static void wireVictimCache(BlockCache l1, BlockCache l2) { + if (l1 instanceof FirstLevelBlockCache) { + try { + ((FirstLevelBlockCache) l1).setVictimCache(l2); + } catch (IllegalArgumentException e) { + // ignore if already wired + } + } + } } 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 03c9f52d37a7..1d7286dbe6c6 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 @@ -21,6 +21,8 @@ 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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -28,12 +30,14 @@ import java.lang.management.ManagementFactory; import java.lang.management.MemoryUsage; import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtil; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.io.ByteBuffAllocator; @@ -48,7 +52,6 @@ import org.apache.hadoop.hbase.testclassification.IOTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.Threads; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -104,7 +107,7 @@ public BlockType getBlockType() { } static class DataCacheEntry implements Cacheable { - private static final int SIZE = 1; + private static final int SIZE = 1 << 20; // 1MB private static DataCacheEntry SINGLETON = new DataCacheEntry(); final CacheableDeserializer deserializer; @@ -307,27 +310,58 @@ private void doBucketCacheConfigTest() { CacheConfig cc = new CacheConfig(this.conf); CacheAccessService service = CacheAccessServiceTestFactory.fromConfiguration(this.conf); basicBlockCacheOps(service, cc, false, false); - assertTrue(CacheAccessServiceTestFactory.blockCache(service) instanceof CombinedBlockCache); + assertTrue(CacheAccessServiceTestFactory.isCombinedBlockCacheEquivalent(service)); // TODO: Assert sizes allocated are right and proportions. - CombinedBlockCache cbc = (CombinedBlockCache) CacheAccessServiceTestFactory.blockCache(service); - BlockCache[] bcs = cbc.getBlockCaches(); - assertTrue(bcs[0] instanceof LruBlockCache); - LruBlockCache lbc = (LruBlockCache) bcs[0]; + LruBlockCache lbc = + (LruBlockCache) CacheAccessServiceTestFactory.getFirstLevelBlockCache(service); + ; assertEquals(MemorySizeUtil.getOnHeapCacheSize(this.conf), lbc.getMaxSize()); - assertTrue(bcs[1] instanceof BucketCache); - BucketCache bc = (BucketCache) bcs[1]; + BucketCache bc = (BucketCache) CacheAccessServiceTestFactory.getSecondLevelBlockCache(service); // getMaxSize comes back in bytes but we specified size in MB assertEquals(bcSize, bc.getMaxSize() / (1024 * 1024)); } /** - * Assert that when BUCKET_CACHE_COMBINED_KEY is false, the non-default, that we deploy - * LruBlockCache as L1 with a BucketCache for L2. + * Verifies the legacy two-tier block cache layout used when bucket cache is enabled but the + * combined-cache mode is disabled. + *

+ * In this configuration HBase should deploy an {@link LruBlockCache} as the first-level in-memory + * cache and a {@link BucketCache} as the second-level victim cache. Blocks are inserted into L1 + * first. When L1 evicts blocks under memory pressure, the evicted blocks should be passed to the + * configured L2 victim cache. + *

+ *

+ * This test intentionally verifies the L1-to-L2 victim-cache relationship without relying on an + * exact final L1 block count or on a specific block key being evicted. {@link LruBlockCache} + * eviction policy does not guarantee which block will be selected for eviction, only that some + * blocks may be evicted when cache pressure exceeds the configured threshold. + *

+ *

+ * The previous version of this test attempted to force eviction by inserting a single synthetic + * block whose size was {@code acceptableSize() + 1}, and then waited until the L1 block count + * returned to its original value. That approach was flawed for two reasons: + *

+ *
    + *
  1. {@link LruBlockCache} rejects any block larger than its maximum cacheable block size before + * eviction can run. Since {@code acceptableSize()} depends on the JVM heap size while the maximum + * cacheable block size is fixed by configuration, {@code acceptableSize() + 1} may be larger than + * the maximum cacheable block size. In that case the block is rejected and no eviction is + * triggered.
    2. + *
  2. If the synthetic block is small enough to be accepted, eviction runs only after the block + * is inserted. The eviction policy is not required to restore the exact previous block count, nor + * is it required to evict the originally inserted block. Waiting for an exact L1 block count can + * therefore hang indefinitely.
    3. + *
+ *

+ * The test now creates cache pressure using normal cacheable blocks and waits with a timeout + * until L2 receives at least one block from L1 eviction. This directly verifies the intended + * contract: L1 is wired with L2 as its victim cache. + *

*/ @Test - public void testBucketCacheConfigL1L2Setup() { + public void testBucketCacheConfigL1L2Setup() throws Exception { this.conf.set(HConstants.BUCKET_CACHE_IOENGINE_KEY, "offheap"); - // Make lru size is smaller than bcSize for sure. Need this to be true so when eviction + // this.conf.setLong("hbase.lru.max.block.size", 1L << 30); // from L1 happens, it does not fail because L2 can't take the eviction because block too big. this.conf.setFloat(HConstants.HFILE_BLOCK_CACHE_SIZE_KEY, 0.001f); MemoryUsage mu = ManagementFactory.getMemoryMXBean().getHeapMemoryUsage(); @@ -339,12 +373,12 @@ public void testBucketCacheConfigL1L2Setup() { CacheConfig cc = new CacheConfig(this.conf); CacheAccessService service = CacheAccessServiceTestFactory.fromConfiguration(this.conf); basicBlockCacheOps(service, cc, false, false); - assertTrue(CacheAccessServiceTestFactory.blockCache(service) instanceof CombinedBlockCache); + assertTrue(CacheAccessServiceTestFactory.isCombinedBlockCacheEquivalent(service)); // TODO: Assert sizes allocated are right and proportions. - CombinedBlockCache cbc = (CombinedBlockCache) CacheAccessServiceTestFactory.blockCache(service); - FirstLevelBlockCache lbc = cbc.l1Cache; + FirstLevelBlockCache lbc = + (FirstLevelBlockCache) CacheAccessServiceTestFactory.getFirstLevelBlockCache(service); assertEquals(lruExpectedSize, lbc.getMaxSize()); - BlockCache bc = cbc.l2Cache; + BlockCache bc = CacheAccessServiceTestFactory.getSecondLevelBlockCache(service); // getMaxSize comes back in bytes but we specified size in MB assertEquals(bcExpectedSize, ((BucketCache) bc).getMaxSize()); // Test the L1+L2 deploy works as we'd expect with blocks evicted from L1 going to L2. @@ -355,24 +389,48 @@ public void testBucketCacheConfigL1L2Setup() { lbc.cacheBlock(bck, c, false); assertEquals(initialL1BlockCount + 1, lbc.getBlockCount()); assertEquals(initialL2BlockCount, bc.getBlockCount()); - // Force evictions by putting in a block too big. - final long justTooBigSize = ((LruBlockCache) lbc).acceptableSize() + 1; - lbc.cacheBlock(new BlockCacheKey("bck2", 0), new DataCacheEntry() { - @Override - public long heapSize() { - return justTooBigSize; - } - - @Override - public int getSerializedLength() { - return (int) heapSize(); - } + + assertNotNull(lbc.getBlock(bck, true, false, true)); + assertNull(bc.getBlock(bck, true, false, true)); + waitForAnyBlockToMoveFromL1ToL2(lbc, bc, initialL2BlockCount); + assertTrue(bc.getBlockCount() > initialL2BlockCount); + } + + /** + * Adds cacheable blocks to L1 until L1 eviction moves at least one block into L2. + *

+ * The helper does not wait for a particular key to appear in L2. {@link LruBlockCache} eviction + * is policy-driven and does not guarantee that the first inserted block, or any specific later + * block, will be evicted first. The observable contract needed by this test is only that an L1 + * eviction is forwarded to the configured L2 victim cache. + *

+ *

+ * This helper also avoids using a single oversized block to force eviction. Oversized blocks may + * be rejected by {@link LruBlockCache} before eviction can run. Instead, it inserts regular + * cacheable blocks and relies on cumulative cache pressure. + *

+ * @param l1Cache first-level cache + * @param l2Cache second-level victim cache + * @param initialL2BlockCount L2 block count before creating L1 pressure + * @throws Exception if the expected L1-to-L2 movement does not happen before the wait timeout + */ + private void waitForAnyBlockToMoveFromL1ToL2(FirstLevelBlockCache l1Cache, BlockCache l2Cache, + long initialL2BlockCount) throws Exception { + AtomicInteger blockIndex = new AtomicInteger(); + + /* + * Do not try to force eviction with one block of size acceptableSize() + 1. LruBlockCache + * rejects blocks larger than maxBlockSize before eviction can run. For accepted blocks, + * eviction runs after insertion and does not guarantee which block will be evicted. Therefore + * this test should not wait for a particular block key to appear in L2. The intended contract + * is only that an L1 eviction moves some evicted block into the configured L2 victim cache. + */ + Waiter.waitFor(this.conf, 10000, () -> { + BlockCacheKey evictionKey = new BlockCacheKey("eviction-" + blockIndex.getAndIncrement(), 0); + l1Cache.cacheBlock(evictionKey, new DataCacheEntry(), false); + + return l2Cache.getBlockCount() > initialL2BlockCount; }); - // The eviction thread in lrublockcache needs to run. - while (initialL1BlockCount != lbc.getBlockCount()) { - Threads.sleep(10); - } - assertEquals(initialL1BlockCount, lbc.getBlockCount()); } @Test 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 0168fa7cc69d..2c6cf46c5b9e 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 @@ -353,7 +353,7 @@ private CacheAccessService initCombinedBlockCacheBackedService(final String l1Ca that.set(BLOCKCACHE_POLICY_KEY, l1CachePolicy); CacheAccessService bc = CacheAccessServiceTestFactory.fromConfiguration(that); assertNotNull(bc); - assertTrue(CacheAccessServiceTestFactory.blockCache(bc) instanceof CombinedBlockCache); + assertTrue(CacheAccessServiceTestFactory.isCombinedBlockCacheEquivalent(bc)); return bc; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServiceTestFactory.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServiceTestFactory.java index 7aec8d9d195d..37dbc0a92278 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServiceTestFactory.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServiceTestFactory.java @@ -669,4 +669,98 @@ public static BlockCache blockCache(CacheAccessService cacheAccessService) { throw new IllegalArgumentException("CacheAccessService is not backed by a legacy BlockCache: " + cacheAccessService.getClass().getName()); } + + /** + * Returns whether the supplied cache access service represents a cache layout that is equivalent + * to the legacy {@link CombinedBlockCache} L1/L2 organization. + *

+ * During the block cache migration, {@link CombinedBlockCache} may no longer be the object used + * by HFile readers and writers directly. The active cache access path may instead be a + * {@link TopologyBackedCacheAccessService} backed by a {@link TieredExclusiveTopology}. That + * topology models the same combined-cache style of exclusive L1/L2 orchestration where the + * participating engines correspond to the legacy L1 and L2 block caches. + *

+ *

+ * This helper is intentionally based on {@link CacheAccessService}, not on the legacy + * {@link BlockCache} field, so callers can reason about the active cache access path during the + * migration away from {@link BlockCache}-centric APIs. + *

+ * @param cacheAccessService cache access service to inspect + * @return {@code true} when the supplied service represents a CombinedBlockCache-compatible + * topology; {@code false} otherwise + */ + public static boolean isCombinedBlockCacheEquivalent(CacheAccessService cacheAccessService) { + if (!(cacheAccessService instanceof TopologyBackedCacheAccessService)) { + return false; + } + + TopologyBackedCacheAccessService topologyBackedService = + (TopologyBackedCacheAccessService) cacheAccessService; + return topologyBackedService.getTopology().getType() == CacheTopologyType.TIERED_EXCLUSIVE; + } + + /** + * Returns the legacy first-level {@link BlockCache} from a topology-backed cache access service. + *

+ * This helper is intended only for tests that need to verify compatibility with the legacy + * {@code CombinedBlockCache} L1/L2 layout after the runtime path has moved to + * {@link TopologyBackedCacheAccessService}. Production code should not unwrap + * {@link CacheAccessService} back to {@link BlockCache}. + *

+ * @param cacheAccessService cache access service to inspect + * @return first-level block cache + */ + public static BlockCache getFirstLevelBlockCache(CacheAccessService cacheAccessService) { + return getBlockCache(cacheAccessService, CacheTier.L1); + } + + /** + * Returns the legacy second-level {@link BlockCache} from a topology-backed cache access service. + *

+ * This helper is intended only for tests that need to verify compatibility with the legacy + * {@code CombinedBlockCache} L1/L2 layout after the runtime path has moved to + * {@link TopologyBackedCacheAccessService}. Production code should not unwrap + * {@link CacheAccessService} back to {@link BlockCache}. + *

+ * @param cacheAccessService cache access service to inspect + * @return second-level block cache + */ + public static BlockCache getSecondLevelBlockCache(CacheAccessService cacheAccessService) { + return getBlockCache(cacheAccessService, CacheTier.L2); + } + + /** + * Returns the legacy {@link BlockCache} backing the requested topology tier. + *

+ * This method expects the supplied {@link CacheAccessService} to be a + * {@link TopologyBackedCacheAccessService} and the requested tier to be backed by a + * {@link BlockCacheBackedCacheEngine}. It is deliberately strict so tests fail clearly when the + * cache access service is not using the expected CombinedBlockCache-compatible topology-backed + * wiring. + *

+ * @param cacheAccessService cache access service to inspect + * @param tier topology tier to unwrap + * @return block cache backing the requested tier + */ + public static BlockCache getBlockCache(CacheAccessService cacheAccessService, CacheTier tier) { + Objects.requireNonNull(cacheAccessService, "cacheAccessService must not be null"); + Objects.requireNonNull(tier, "tier must not be null"); + + if (!(cacheAccessService instanceof TopologyBackedCacheAccessService)) { + throw new IllegalArgumentException( + "cacheAccessService must be a TopologyBackedCacheAccessService"); + } + + TopologyBackedCacheAccessService topologyBackedService = + (TopologyBackedCacheAccessService) cacheAccessService; + CacheEngine engine = topologyBackedService.getTopology().getEngine(tier) + .orElseThrow(() -> new IllegalArgumentException("No cache engine found for tier " + tier)); + + if (!(engine instanceof BlockCacheBackedCacheEngine)) { + throw new IllegalArgumentException( + "Cache engine for tier " + tier + " must be a BlockCacheBackedCacheEngine"); + } + + return ((BlockCacheBackedCacheEngine) engine).getBlockCache(); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCacheAccessServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCacheAccessServices.java new file mode 100644 index 000000000000..1c548b9f9304 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCacheAccessServices.java @@ -0,0 +1,83 @@ +/* + * 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.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Optional; +import org.apache.hadoop.hbase.io.hfile.BlockCache; +import org.apache.hadoop.hbase.io.hfile.CombinedBlockCache; +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; + +@Tag(IOTests.TAG) +@Tag(SmallTests.TAG) +public class TestCacheAccessServices { + + @Test + void testFromBlockCacheCreatesBlockCacheBackedServiceForRegularBlockCache() { + BlockCache blockCache = mock(BlockCache.class); + CacheAccessService service = CacheAccessServices.fromBlockCache(blockCache); + assertTrue(service instanceof BlockCacheBackedCacheAccessService); + } + + @Test + void testFromBlockCacheCreatesTopologyBackedServiceForCombinedBlockCache() { + BlockCache l1 = mock(BlockCache.class); + BlockCache l2 = mock(BlockCache.class); + CombinedBlockCache combinedBlockCache = mock(CombinedBlockCache.class); + + when(combinedBlockCache.getBlockCaches()).thenReturn(new BlockCache[] { l1, l2 }); + + CacheAccessService service = CacheAccessServices.fromBlockCache(combinedBlockCache); + + assertTrue(service instanceof TopologyBackedCacheAccessService); + + TopologyBackedCacheAccessService topologyBackedService = + (TopologyBackedCacheAccessService) service; + + assertTrue(topologyBackedService.getTopology() instanceof TieredExclusiveTopology); + assertSame(CacheTopologyType.TIERED_EXCLUSIVE, topologyBackedService.getTopology().getType()); + + Optional l1Engine = topologyBackedService.getTopology().getEngine(CacheTier.L1); + Optional l2Engine = topologyBackedService.getTopology().getEngine(CacheTier.L2); + + assertTrue(l1Engine.isPresent()); + assertTrue(l2Engine.isPresent()); + assertTrue(l1Engine.get() instanceof BlockCacheBackedCacheEngine); + assertTrue(l2Engine.get() instanceof BlockCacheBackedCacheEngine); + assertSame(l1, ((BlockCacheBackedCacheEngine) l1Engine.get()).getBlockCache()); + assertSame(l2, ((BlockCacheBackedCacheEngine) l2Engine.get()).getBlockCache()); + } + + @Test + void testFromBlockCacheRejectsNull() { + assertThrows(NullPointerException.class, () -> CacheAccessServices.fromBlockCache(null)); + } + + @Test + void testDisabledReturnsNoOpService() { + assertTrue(CacheAccessServices.disabled() instanceof NoOpCacheAccessService); + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCombinedBlockCacheCompatibleTopologyBackedCacheAccessService.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCombinedBlockCacheCompatibleTopologyBackedCacheAccessService.java index 58db25bb9dc7..209ceeae2028 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCombinedBlockCacheCompatibleTopologyBackedCacheAccessService.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestCombinedBlockCacheCompatibleTopologyBackedCacheAccessService.java @@ -30,6 +30,7 @@ import static org.mockito.Mockito.when; import java.util.Arrays; +import java.util.Optional; import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.BlockCacheKey; import org.apache.hadoop.hbase.io.hfile.Cacheable; @@ -50,12 +51,14 @@ void testL1HitReturnsBlockWithoutCheckingL2() { BlockCacheKey key = new BlockCacheKey("file", 1L); Cacheable block = mock(Cacheable.class); + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(true)); when(l1.getBlock(key, true, false, true)).thenReturn(block); TopologyBackedCacheAccessService service = service(l1, l2, noPromotionPolicy()); assertSame(block, service.getBlock(key, requestContext())); + verify(l1).isAlreadyCached(key); verify(l1).getBlock(key, true, false, true); verify(l2, never()).getBlock(any(), anyBoolean(), anyBoolean(), anyBoolean()); } @@ -66,15 +69,14 @@ void testL2HitReturnsBlock() { BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); Cacheable block = mock(Cacheable.class); - + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(false)); when(l1.getBlock(key, true, false, true)).thenReturn(null); when(l2.getBlock(key, true, false, true)).thenReturn(block); TopologyBackedCacheAccessService service = service(l1, l2, noPromotionPolicy()); assertSame(block, service.getBlock(key, requestContext())); - - verify(l1).getBlock(key, true, false, true); + verify(l1).isAlreadyCached(key); verify(l2).getBlock(key, true, false, true); } @@ -83,15 +85,14 @@ void testMissReturnsNull() { BlockCache l1 = mock(BlockCache.class); BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); - + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(false)); when(l1.getBlock(key, true, false, true)).thenReturn(null); when(l2.getBlock(key, true, false, true)).thenReturn(null); TopologyBackedCacheAccessService service = service(l1, l2, noPromotionPolicy()); assertNull(service.getBlock(key, requestContext())); - - verify(l1).getBlock(key, true, false, true); + verify(l1).isAlreadyCached(key); verify(l2).getBlock(key, true, false, true); } @@ -101,7 +102,7 @@ void testL2HitWithPromotionMovesBlockToL1() { BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); Cacheable block = mock(Cacheable.class); - + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(false)); when(l1.getBlock(key, true, false, true)).thenReturn(null); when(l2.getBlock(key, true, false, true)).thenReturn(block); @@ -109,7 +110,7 @@ void testL2HitWithPromotionMovesBlockToL1() { assertSame(block, service.getBlock(key, requestContext())); - verify(l1).getBlock(key, true, false, true); + verify(l1).isAlreadyCached(key); verify(l2).getBlock(key, true, false, true); assertCachedExactlyOnce(l1, key, block); verify(l2).evictBlock(key); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServiceWithBlockCacheBackedEngines.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServiceWithBlockCacheBackedEngines.java index 02d39623103f..38614cb81db2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServiceWithBlockCacheBackedEngines.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServiceWithBlockCacheBackedEngines.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.when; import java.util.Arrays; +import java.util.Optional; import org.apache.hadoop.hbase.io.hfile.BlockCache; import org.apache.hadoop.hbase.io.hfile.BlockCacheKey; import org.apache.hadoop.hbase.io.hfile.Cacheable; @@ -45,13 +46,14 @@ void testL1HitDoesNotCheckL2() { BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); Cacheable block = mock(Cacheable.class); + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(true)); when(l1.getBlock(key, true, false, true)).thenReturn(block); TopologyBackedCacheAccessService service = service(l1, l2, noPromotionPolicy()); assertSame(block, service.getBlock(key, requestContext())); - + verify(l1).isAlreadyCached(key); verify(l1).getBlock(key, true, false, true); verify(l2, never()).getBlock(any(), any(Boolean.class), any(Boolean.class), any(Boolean.class)); } @@ -62,7 +64,7 @@ void testL2HitPromotesToL1AndEvictsFromL2() { BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); Cacheable block = mock(Cacheable.class); - + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(false)); when(l1.getBlock(key, true, false, true)).thenReturn(null); when(l2.getBlock(key, true, false, true)).thenReturn(block); @@ -70,7 +72,8 @@ void testL2HitPromotesToL1AndEvictsFromL2() { assertSame(block, service.getBlock(key, requestContext())); - verify(l1).getBlock(key, true, false, true); + verify(l1, never()).getBlock(any(), any(Boolean.class), any(Boolean.class), any(Boolean.class)); + verify(l1).isAlreadyCached(key); verify(l2).getBlock(key, true, false, true); verify(l1).cacheBlock(key, block); verify(l2).evictBlock(key); @@ -81,15 +84,15 @@ void testMissReturnsNull() { BlockCache l1 = mock(BlockCache.class); BlockCache l2 = mock(BlockCache.class); BlockCacheKey key = new BlockCacheKey("file", 1L); - + when(l1.isAlreadyCached(key)).thenReturn(Optional.of(false)); when(l1.getBlock(key, true, false, true)).thenReturn(null); when(l2.getBlock(key, true, false, true)).thenReturn(null); TopologyBackedCacheAccessService service = service(l1, l2, noPromotionPolicy()); assertNull(service.getBlock(key, requestContext())); - - verify(l1).getBlock(key, true, false, true); + verify(l1, never()).getBlock(any(), any(Boolean.class), any(Boolean.class), any(Boolean.class)); + verify(l1).isAlreadyCached(key); verify(l2).getBlock(key, true, false, true); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServices.java index 8869d30cb55d..1d81c0e44626 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/cache/TestTopologyBackedCacheAccessServices.java @@ -22,9 +22,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.Optional; import org.apache.hadoop.hbase.io.hfile.BlockCache; +import org.apache.hadoop.hbase.io.hfile.CombinedBlockCache; import org.apache.hadoop.hbase.testclassification.IOTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.jupiter.api.Tag; @@ -74,4 +76,56 @@ void testFromTieredExclusiveBlockCachesRejectsNullArguments() { assertThrows(NullPointerException.class, () -> TopologyBackedCacheAccessServices .fromTieredExclusiveBlockCaches("combined", l1, l2, null)); } + + @Test + void testFromCombinedBlockCacheCreatesExpectedService() { + BlockCache l1 = mock(BlockCache.class); + BlockCache l2 = mock(BlockCache.class); + CombinedBlockCache combinedBlockCache = mock(CombinedBlockCache.class); + CachePlacementAdmissionPolicy policy = mock(CachePlacementAdmissionPolicy.class); + + when(combinedBlockCache.getBlockCaches()).thenReturn(new BlockCache[] { l1, l2 }); + + TopologyBackedCacheAccessService service = + TopologyBackedCacheAccessServices.fromCombinedBlockCache(combinedBlockCache, policy); + + assertEquals("combined", service.getName()); + assertSame(policy, service.getPolicy()); + assertTrue(service.getTopology() instanceof TieredExclusiveTopology); + assertEquals(CacheTopologyType.TIERED_EXCLUSIVE, service.getTopology().getType()); + + Optional l1Engine = service.getTopology().getEngine(CacheTier.L1); + Optional l2Engine = service.getTopology().getEngine(CacheTier.L2); + + assertTrue(l1Engine.isPresent()); + assertTrue(l2Engine.isPresent()); + assertTrue(l1Engine.get() instanceof BlockCacheBackedCacheEngine); + assertTrue(l2Engine.get() instanceof BlockCacheBackedCacheEngine); + assertSame(l1, ((BlockCacheBackedCacheEngine) l1Engine.get()).getBlockCache()); + assertSame(l2, ((BlockCacheBackedCacheEngine) l2Engine.get()).getBlockCache()); + } + + @Test + void testFromCombinedBlockCacheRejectsNullCombinedBlockCache() { + CachePlacementAdmissionPolicy policy = mock(CachePlacementAdmissionPolicy.class); + assertThrows(NullPointerException.class, + () -> TopologyBackedCacheAccessServices.fromCombinedBlockCache(null, policy)); + } + + @Test + void testFromCombinedBlockCacheRejectsNullPolicy() { + CombinedBlockCache combinedBlockCache = mock(CombinedBlockCache.class); + assertThrows(NullPointerException.class, + () -> TopologyBackedCacheAccessServices.fromCombinedBlockCache(combinedBlockCache, null)); + } + + @Test + void testFromCombinedBlockCacheRejectsUnexpectedTierCount() { + BlockCache l1 = mock(BlockCache.class); + CombinedBlockCache combinedBlockCache = mock(CombinedBlockCache.class); + CachePlacementAdmissionPolicy policy = mock(CachePlacementAdmissionPolicy.class); + when(combinedBlockCache.getBlockCaches()).thenReturn(new BlockCache[] { l1 }); + assertThrows(IllegalArgumentException.class, + () -> TopologyBackedCacheAccessServices.fromCombinedBlockCache(combinedBlockCache, policy)); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java index 9bfe848d9682..4e437d723465 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java @@ -773,7 +773,6 @@ private void validateBlocks(Set keys, int expectedTotalKeys, int int numHotBlocks = 0, numColdBlocks = 0; Waiter.waitFor(defaultConf, 10000, 100, () -> (expectedTotalKeys == keys.size())); - int iter = 0; for (BlockCacheKey key : keys) { try { if (dataTieringManager.isHotData(key)) { @@ -805,11 +804,6 @@ private void testDataTieringMethodWithKey(DataTieringMethodCallerWithKey caller, } } - private void testDataTieringMethodWithKeyExpectingException(DataTieringMethodCallerWithKey caller, - BlockCacheKey key, DataTieringException exception) { - testDataTieringMethodWithKey(caller, key, false, exception); - } - private void testDataTieringMethodWithKeyNoException(DataTieringMethodCallerWithKey caller, BlockCacheKey key, boolean expectedResult) { testDataTieringMethodWithKey(caller, key, expectedResult, null);