From 77209a79d6c207dc1ee0092f102fa5938a4948f6 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Wed, 9 Apr 2025 19:09:16 +0100 Subject: [PATCH 1/6] HBASE-29249 Allow for BlockCache implementations to define dynamic properties Change-Id: I3681c3b1a50196a70ece04fde08da06448d16353 --- .../hadoop/hbase/io/hfile/BlockCache.java | 11 +++ .../hadoop/hbase/io/hfile/CacheConfig.java | 1 + .../hbase/io/hfile/CombinedBlockCache.java | 7 ++ .../hbase/io/hfile/bucket/BucketCache.java | 57 +++++++---- .../io/hfile/bucket/TestBucketCache.java | 94 ++++++++++++++----- 5 files changed, 128 insertions(+), 42 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java index 8431dbf25a80..b4bd3239435e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java @@ -20,6 +20,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Optional; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; @@ -279,4 +280,14 @@ default boolean isCacheEnabled() { default boolean waitForCacheInitialization(long timeout) { return true; } + + /** + * Allows for BlockCache implementations to provide a mean to refresh their configurations. Since + * HBASE-28517, CacheConfig implements ConfigurationObserver and registers itself for + * notifications of dynamic configuration changes. The default is a noop. + * @param config the new configuration to be updated. + */ + default void refreshConfiguration(Configuration config) { + // noop + } } 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 78f62bfc77ff..3872d0463bb1 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 @@ -479,5 +479,6 @@ public void onConfigurationChange(Configuration conf) { + "hbase.rs.cacheblocksonwrite is changed to {}, " + "hbase.rs.evictblocksonclose is changed to {}", cacheDataOnRead, cacheDataOnWrite, evictOnClose); + blockCache.refreshConfiguration(conf); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java index caf5c374f395..36332557d214 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java @@ -22,6 +22,7 @@ import java.util.Optional; import org.apache.commons.lang3.mutable.Mutable; import org.apache.commons.lang3.mutable.MutableBoolean; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.io.HeapSize; import org.apache.hadoop.hbase.io.hfile.bucket.BucketCache; @@ -470,6 +471,12 @@ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int d } + @Override + public void refreshConfiguration(Configuration config) { + l1Cache.refreshConfiguration(config); + l2Cache.refreshConfiguration(config); + } + @Override public Optional blockFitsIntoTheCache(HFileBlock block) { if (isMetaBlock(block.getBlockType())) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java index 6388b5ea7dae..7aef9072b4e0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java @@ -144,11 +144,11 @@ public class BucketCache implements BlockCache, HeapSize { static final float DEFAULT_MEMORY_FACTOR = 0.25f; static final float DEFAULT_MIN_FACTOR = 0.85f; - private static final float DEFAULT_EXTRA_FREE_FACTOR = 0.10f; - private static final float DEFAULT_ACCEPT_FACTOR = 0.95f; + static final float DEFAULT_EXTRA_FREE_FACTOR = 0.10f; + static final float DEFAULT_ACCEPT_FACTOR = 0.95f; // Number of blocks to clear for each of the bucket size that is full - private static final int DEFAULT_FREE_ENTIRE_BLOCK_FACTOR = 2; + static final int DEFAULT_FREE_ENTIRE_BLOCK_FACTOR = 2; /** Statistics thread */ private static final int statThreadPeriod = 5 * 60; @@ -289,7 +289,7 @@ protected enum CacheState { private static final String DEFAULT_FILE_VERIFY_ALGORITHM = "MD5"; public static final String QUEUE_ADDITION_WAIT_TIME = "hbase.bucketcache.queue.addition.waittime"; - private static final long DEFAULT_QUEUE_ADDITION_WAIT_TIME = 0; + static final long DEFAULT_QUEUE_ADDITION_WAIT_TIME = 0; private long queueAdditionWaitTime; /** * Use {@link java.security.MessageDigest} class's encryption algorithms to check persistent file @@ -344,22 +344,8 @@ public BucketCache(String ioEngineName, long capacity, int blockSize, int[] buck throw new IllegalArgumentException("Cache capacity is too large, only support 32TB now"); } - this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME, DEFAULT_ACCEPT_FACTOR); - this.minFactor = conf.getFloat(MIN_FACTOR_CONFIG_NAME, DEFAULT_MIN_FACTOR); - this.extraFreeFactor = conf.getFloat(EXTRA_FREE_FACTOR_CONFIG_NAME, DEFAULT_EXTRA_FREE_FACTOR); - this.singleFactor = conf.getFloat(SINGLE_FACTOR_CONFIG_NAME, DEFAULT_SINGLE_FACTOR); - this.multiFactor = conf.getFloat(MULTI_FACTOR_CONFIG_NAME, DEFAULT_MULTI_FACTOR); - this.memoryFactor = conf.getFloat(MEMORY_FACTOR_CONFIG_NAME, DEFAULT_MEMORY_FACTOR); - this.queueAdditionWaitTime = - conf.getLong(QUEUE_ADDITION_WAIT_TIME, DEFAULT_QUEUE_ADDITION_WAIT_TIME); - this.bucketcachePersistInterval = conf.getLong(BUCKETCACHE_PERSIST_INTERVAL_KEY, 1000); - this.persistenceChunkSize = - conf.getLong(BACKING_MAP_PERSISTENCE_CHUNK_SIZE, DEFAULT_BACKING_MAP_PERSISTENCE_CHUNK_SIZE); - if (this.persistenceChunkSize <= 0) { - persistenceChunkSize = DEFAULT_BACKING_MAP_PERSISTENCE_CHUNK_SIZE; - } - - sanityCheckConfigs(); + // these sets the dynamic configs + this.refreshConfiguration(conf); LOG.info("Instantiating BucketCache with acceptableFactor: " + acceptableFactor + ", minFactor: " + minFactor + ", extraFreeFactor: " + extraFreeFactor + ", singleFactor: " @@ -450,6 +436,9 @@ private void sanityCheckConfigs() { Preconditions.checkArgument((singleFactor + multiFactor + memoryFactor) == 1, SINGLE_FACTOR_CONFIG_NAME + ", " + MULTI_FACTOR_CONFIG_NAME + ", and " + MEMORY_FACTOR_CONFIG_NAME + " segments must add up to 1.0"); + if (this.persistenceChunkSize <= 0) { + persistenceChunkSize = DEFAULT_BACKING_MAP_PERSISTENCE_CHUNK_SIZE; + } } /** @@ -911,6 +900,22 @@ boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey, BucketEnt return false; } + @Override + public void refreshConfiguration(Configuration config) { + this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME, DEFAULT_ACCEPT_FACTOR); + this.minFactor = conf.getFloat(MIN_FACTOR_CONFIG_NAME, DEFAULT_MIN_FACTOR); + this.extraFreeFactor = conf.getFloat(EXTRA_FREE_FACTOR_CONFIG_NAME, DEFAULT_EXTRA_FREE_FACTOR); + this.singleFactor = conf.getFloat(SINGLE_FACTOR_CONFIG_NAME, DEFAULT_SINGLE_FACTOR); + this.multiFactor = conf.getFloat(MULTI_FACTOR_CONFIG_NAME, DEFAULT_MULTI_FACTOR); + this.memoryFactor = conf.getFloat(MEMORY_FACTOR_CONFIG_NAME, DEFAULT_MEMORY_FACTOR); + this.queueAdditionWaitTime = + conf.getLong(QUEUE_ADDITION_WAIT_TIME, DEFAULT_QUEUE_ADDITION_WAIT_TIME); + this.bucketcachePersistInterval = conf.getLong(BUCKETCACHE_PERSIST_INTERVAL_KEY, 1000); + this.persistenceChunkSize = + conf.getLong(BACKING_MAP_PERSISTENCE_CHUNK_SIZE, DEFAULT_BACKING_MAP_PERSISTENCE_CHUNK_SIZE); + sanityCheckConfigs(); + } + protected boolean removeFromRamCache(BlockCacheKey cacheKey) { return ramCache.remove(cacheKey, re -> { if (re != null) { @@ -2157,6 +2162,18 @@ float getMemoryFactor() { return memoryFactor; } + long getQueueAdditionWaitTime() { + return queueAdditionWaitTime; + } + + long getPersistenceChunkSize() { + return persistenceChunkSize; + } + + long getBucketcachePersistInterval() { + return bucketcachePersistInterval; + } + public String getPersistencePath() { return persistencePath; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java index 67e498364efc..8f3230097c78 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java @@ -17,12 +17,19 @@ */ package org.apache.hadoop.hbase.io.hfile.bucket; +import static org.apache.hadoop.hbase.io.hfile.CacheConfig.BUCKETCACHE_PERSIST_INTERVAL_KEY; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.ACCEPT_FACTOR_CONFIG_NAME; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.BACKING_MAP_PERSISTENCE_CHUNK_SIZE; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.BLOCK_ORPHAN_GRACE_PERIOD; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.DEFAULT_ERROR_TOLERATION_DURATION; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.DEFAULT_MIN_FACTOR; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.DEFAULT_SINGLE_FACTOR; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.EXTRA_FREE_FACTOR_CONFIG_NAME; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.MEMORY_FACTOR_CONFIG_NAME; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.MIN_FACTOR_CONFIG_NAME; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.MULTI_FACTOR_CONFIG_NAME; import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.QUEUE_ADDITION_WAIT_TIME; +import static org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.SINGLE_FACTOR_CONFIG_NAME; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -472,14 +479,13 @@ public void testBucketAllocatorLargeBuckets() throws BucketAllocatorException { @Test public void testGetPartitionSize() throws IOException { // Test default values - validateGetPartitionSize(cache, BucketCache.DEFAULT_SINGLE_FACTOR, - BucketCache.DEFAULT_MIN_FACTOR); + validateGetPartitionSize(cache, DEFAULT_SINGLE_FACTOR, DEFAULT_MIN_FACTOR); Configuration conf = HBaseConfiguration.create(); conf.setFloat(MIN_FACTOR_CONFIG_NAME, 0.5f); - conf.setFloat(BucketCache.SINGLE_FACTOR_CONFIG_NAME, 0.1f); - conf.setFloat(BucketCache.MULTI_FACTOR_CONFIG_NAME, 0.7f); - conf.setFloat(BucketCache.MEMORY_FACTOR_CONFIG_NAME, 0.2f); + conf.setFloat(SINGLE_FACTOR_CONFIG_NAME, 0.1f); + conf.setFloat(MULTI_FACTOR_CONFIG_NAME, 0.7f); + conf.setFloat(MEMORY_FACTOR_CONFIG_NAME, 0.2f); BucketCache cache = new BucketCache(ioEngineName, capacitySize, constructedBlockSize, constructedBlockSizes, writeThreads, writerQLen, null, 100, conf); @@ -493,13 +499,12 @@ public void testGetPartitionSize() throws IOException { @Test public void testCacheSizeCapacity() throws IOException { // Test cache capacity (capacity / blockSize) < Integer.MAX_VALUE - validateGetPartitionSize(cache, BucketCache.DEFAULT_SINGLE_FACTOR, - BucketCache.DEFAULT_MIN_FACTOR); + validateGetPartitionSize(cache, DEFAULT_SINGLE_FACTOR, DEFAULT_MIN_FACTOR); Configuration conf = HBaseConfiguration.create(); conf.setFloat(BucketCache.MIN_FACTOR_CONFIG_NAME, 0.5f); - conf.setFloat(BucketCache.SINGLE_FACTOR_CONFIG_NAME, 0.1f); - conf.setFloat(BucketCache.MULTI_FACTOR_CONFIG_NAME, 0.7f); - conf.setFloat(BucketCache.MEMORY_FACTOR_CONFIG_NAME, 0.2f); + conf.setFloat(SINGLE_FACTOR_CONFIG_NAME, 0.1f); + conf.setFloat(MULTI_FACTOR_CONFIG_NAME, 0.7f); + conf.setFloat(MEMORY_FACTOR_CONFIG_NAME, 0.2f); try { new BucketCache(ioEngineName, Long.MAX_VALUE, 1, constructedBlockSizes, writeThreads, writerQLen, null, 100, conf); @@ -515,9 +520,9 @@ public void testValidBucketCacheConfigs() throws IOException { conf.setFloat(ACCEPT_FACTOR_CONFIG_NAME, 0.9f); conf.setFloat(MIN_FACTOR_CONFIG_NAME, 0.5f); conf.setFloat(EXTRA_FREE_FACTOR_CONFIG_NAME, 0.5f); - conf.setFloat(BucketCache.SINGLE_FACTOR_CONFIG_NAME, 0.1f); - conf.setFloat(BucketCache.MULTI_FACTOR_CONFIG_NAME, 0.7f); - conf.setFloat(BucketCache.MEMORY_FACTOR_CONFIG_NAME, 0.2f); + conf.setFloat(SINGLE_FACTOR_CONFIG_NAME, 0.1f); + conf.setFloat(MULTI_FACTOR_CONFIG_NAME, 0.7f); + conf.setFloat(MEMORY_FACTOR_CONFIG_NAME, 0.2f); BucketCache cache = new BucketCache(ioEngineName, capacitySize, constructedBlockSize, constructedBlockSizes, writeThreads, writerQLen, null, 100, conf); @@ -528,12 +533,12 @@ public void testValidBucketCacheConfigs() throws IOException { assertEquals(MIN_FACTOR_CONFIG_NAME + " failed to propagate.", 0.5f, cache.getMinFactor(), 0); assertEquals(EXTRA_FREE_FACTOR_CONFIG_NAME + " failed to propagate.", 0.5f, cache.getExtraFreeFactor(), 0); - assertEquals(BucketCache.SINGLE_FACTOR_CONFIG_NAME + " failed to propagate.", 0.1f, - cache.getSingleFactor(), 0); - assertEquals(BucketCache.MULTI_FACTOR_CONFIG_NAME + " failed to propagate.", 0.7f, - cache.getMultiFactor(), 0); - assertEquals(BucketCache.MEMORY_FACTOR_CONFIG_NAME + " failed to propagate.", 0.2f, - cache.getMemoryFactor(), 0); + assertEquals(SINGLE_FACTOR_CONFIG_NAME + " failed to propagate.", 0.1f, cache.getSingleFactor(), + 0); + assertEquals(MULTI_FACTOR_CONFIG_NAME + " failed to propagate.", 0.7f, cache.getMultiFactor(), + 0); + assertEquals(MEMORY_FACTOR_CONFIG_NAME + " failed to propagate.", 0.2f, cache.getMemoryFactor(), + 0); } @Test @@ -575,9 +580,9 @@ public void testInvalidCacheSplitFactorConfig() throws IOException { // be negative, configs don't add to 1.0 boolean[] expectedOutcomes = { true, false, false, false }; Map configMappings = ImmutableMap.of(BucketCache.SINGLE_FACTOR_CONFIG_NAME, - singleFactorConfigValues, BucketCache.MULTI_FACTOR_CONFIG_NAME, multiFactorConfigValues, - BucketCache.MEMORY_FACTOR_CONFIG_NAME, memoryFactorConfigValues); + float[]> configMappings = ImmutableMap.of(SINGLE_FACTOR_CONFIG_NAME, singleFactorConfigValues, + MULTI_FACTOR_CONFIG_NAME, multiFactorConfigValues, MEMORY_FACTOR_CONFIG_NAME, + memoryFactorConfigValues); Configuration conf = HBaseConfiguration.create(); checkConfigValues(conf, configMappings, expectedOutcomes); } @@ -921,6 +926,51 @@ public void testBlockAdditionWaitWhenCache() throws Exception { } } + @Test + public void testOnConfigurationChange() throws Exception { + BucketCache bucketCache = null; + try { + final Path dataTestDir = createAndGetTestDir(); + + String ioEngineName = "file:" + dataTestDir + "/bucketNoRecycler.cache"; + + Configuration config = HBASE_TESTING_UTILITY.getConfiguration(); + + bucketCache = new BucketCache(ioEngineName, capacitySize, constructedBlockSize, + constructedBlockSizes, 1, 1, null, DEFAULT_ERROR_TOLERATION_DURATION, config); + + assertTrue(bucketCache.waitForCacheInitialization(10000)); + + config.setFloat(ACCEPT_FACTOR_CONFIG_NAME, 0.9f); + config.setFloat(MIN_FACTOR_CONFIG_NAME, 0.8f); + config.setFloat(EXTRA_FREE_FACTOR_CONFIG_NAME, 0.15f); + config.setFloat(SINGLE_FACTOR_CONFIG_NAME, 0.2f); + config.setFloat(MULTI_FACTOR_CONFIG_NAME, 0.6f); + config.setFloat(MEMORY_FACTOR_CONFIG_NAME, 0.2f); + config.setLong(QUEUE_ADDITION_WAIT_TIME, 100); + config.setLong(BUCKETCACHE_PERSIST_INTERVAL_KEY, 500); + config.setLong(BACKING_MAP_PERSISTENCE_CHUNK_SIZE, 1000); + + bucketCache.refreshConfiguration(config); + + assertEquals(0.9f, bucketCache.getAcceptableFactor(), 0.01); + assertEquals(0.8f, bucketCache.getMinFactor(), 0.01); + assertEquals(0.15f, bucketCache.getExtraFreeFactor(), 0.01); + assertEquals(0.2f, bucketCache.getSingleFactor(), 0.01); + assertEquals(0.6f, bucketCache.getMultiFactor(), 0.01); + assertEquals(0.2f, bucketCache.getMemoryFactor(), 0.01); + assertEquals(100L, bucketCache.getQueueAdditionWaitTime()); + assertEquals(500L, bucketCache.getBucketcachePersistInterval()); + assertEquals(1000L, bucketCache.getPersistenceChunkSize()); + + } finally { + if (bucketCache != null) { + bucketCache.shutdown(); + } + HBASE_TESTING_UTILITY.cleanupTestDir(); + } + } + @Test public void testNotifyFileCachingCompletedSuccess() throws Exception { BucketCache bucketCache = null; From 7f14a1aa683669a0cf967b6646eb342a5ebe93a0 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Thu, 10 Apr 2025 12:43:36 +0100 Subject: [PATCH 2/6] Addressing review comments Change-Id: I0aac16ed7d685ac533fe9d0325747be03adc9140 --- .../apache/hadoop/hbase/io/hfile/BlockCache.java | 10 ++++++---- .../apache/hadoop/hbase/io/hfile/CacheConfig.java | 15 +++++++++++++-- .../hadoop/hbase/io/hfile/CombinedBlockCache.java | 6 +++--- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 4 ++-- .../hbase/io/hfile/bucket/TestBucketCache.java | 2 +- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java index b4bd3239435e..378e28e9b5d2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java @@ -22,6 +22,7 @@ import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.conf.ConfigurationObserver; import org.apache.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; @@ -30,7 +31,7 @@ * cache. */ @InterfaceAudience.Private -public interface BlockCache extends Iterable { +public interface BlockCache extends Iterable, ConfigurationObserver { /** * Add block to cache. * @param cacheKey The block's cache key. @@ -283,11 +284,12 @@ default boolean waitForCacheInitialization(long timeout) { /** * Allows for BlockCache implementations to provide a mean to refresh their configurations. Since - * HBASE-28517, CacheConfig implements ConfigurationObserver and registers itself for - * notifications of dynamic configuration changes. The default is a noop. + * HBASE-29249, CacheConfig implements PropagatingConfigurationObserver and registers itself + * together with the used BlockCache implementation for notifications of dynamic configuration + * changes. The default is a noop. * @param config the new configuration to be updated. */ - default void refreshConfiguration(Configuration config) { + default void onConfigurationChange(Configuration config) { // noop } } 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 3872d0463bb1..5b49983b8ebc 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 @@ -20,7 +20,9 @@ import java.util.Optional; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; +import org.apache.hadoop.hbase.conf.ConfigurationManager; import org.apache.hadoop.hbase.conf.ConfigurationObserver; +import org.apache.hadoop.hbase.conf.PropagatingConfigurationObserver; import org.apache.hadoop.hbase.io.ByteBuffAllocator; import org.apache.hadoop.hbase.io.hfile.BlockType.BlockCategory; import org.apache.yetus.audience.InterfaceAudience; @@ -31,7 +33,7 @@ * Stores all of the cache objects and configuration for a single HFile. */ @InterfaceAudience.Private -public class CacheConfig implements ConfigurationObserver { +public class CacheConfig implements PropagatingConfigurationObserver { private static final Logger LOG = LoggerFactory.getLogger(CacheConfig.class.getName()); /** @@ -479,6 +481,15 @@ public void onConfigurationChange(Configuration conf) { + "hbase.rs.cacheblocksonwrite is changed to {}, " + "hbase.rs.evictblocksonclose is changed to {}", cacheDataOnRead, cacheDataOnWrite, evictOnClose); - blockCache.refreshConfiguration(conf); + } + + @Override + public void registerChildren(ConfigurationManager manager) { + manager.registerObserver(blockCache); + } + + @Override + public void deregisterChildren(ConfigurationManager manager) { + manager.deregisterObserver(blockCache); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java index 36332557d214..7e503f99f3f6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java @@ -472,9 +472,9 @@ public void notifyFileCachingCompleted(Path fileName, int totalBlockCount, int d } @Override - public void refreshConfiguration(Configuration config) { - l1Cache.refreshConfiguration(config); - l2Cache.refreshConfiguration(config); + public void onConfigurationChange(Configuration config) { + l1Cache.onConfigurationChange(config); + l2Cache.onConfigurationChange(config); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java index 7aef9072b4e0..e4ff80b1b3f4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java @@ -345,7 +345,7 @@ public BucketCache(String ioEngineName, long capacity, int blockSize, int[] buck } // these sets the dynamic configs - this.refreshConfiguration(conf); + this.onConfigurationChange(conf); LOG.info("Instantiating BucketCache with acceptableFactor: " + acceptableFactor + ", minFactor: " + minFactor + ", extraFreeFactor: " + extraFreeFactor + ", singleFactor: " @@ -901,7 +901,7 @@ boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey, BucketEnt } @Override - public void refreshConfiguration(Configuration config) { + public void onConfigurationChange(Configuration config) { this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME, DEFAULT_ACCEPT_FACTOR); this.minFactor = conf.getFloat(MIN_FACTOR_CONFIG_NAME, DEFAULT_MIN_FACTOR); this.extraFreeFactor = conf.getFloat(EXTRA_FREE_FACTOR_CONFIG_NAME, DEFAULT_EXTRA_FREE_FACTOR); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java index 8f3230097c78..93703931ed2c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java @@ -951,7 +951,7 @@ public void testOnConfigurationChange() throws Exception { config.setLong(BUCKETCACHE_PERSIST_INTERVAL_KEY, 500); config.setLong(BACKING_MAP_PERSISTENCE_CHUNK_SIZE, 1000); - bucketCache.refreshConfiguration(config); + bucketCache.onConfigurationChange(config); assertEquals(0.9f, bucketCache.getAcceptableFactor(), 0.01); assertEquals(0.8f, bucketCache.getMinFactor(), 0.01); From 378dc2f98129a8801f154e20026b330c423f3e59 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Thu, 10 Apr 2025 12:44:53 +0100 Subject: [PATCH 3/6] temp Change-Id: I72430e198d729500cd314ea1ec59d05edb8f415a --- .../main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java | 2 +- .../main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java index 378e28e9b5d2..43d1e77c7e67 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java @@ -31,7 +31,7 @@ * cache. */ @InterfaceAudience.Private -public interface BlockCache extends Iterable, ConfigurationObserver { +public interface BlockCache extends Iterable, ConfigurationObserver { /** * Add block to cache. * @param cacheKey The block's cache key. 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 5b49983b8ebc..4fadd812ec56 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 @@ -21,7 +21,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.conf.ConfigurationManager; -import org.apache.hadoop.hbase.conf.ConfigurationObserver; import org.apache.hadoop.hbase.conf.PropagatingConfigurationObserver; import org.apache.hadoop.hbase.io.ByteBuffAllocator; import org.apache.hadoop.hbase.io.hfile.BlockType.BlockCategory; From 71cead78adf9f2356e19e11ebbe28ca0002479d6 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Thu, 10 Apr 2025 18:03:58 +0100 Subject: [PATCH 4/6] trying to address flakeyness (not related to the original change) Change-Id: I12afe38cd3df15974a44101accd74614edec2a01 --- .../hbase/io/hfile/TestBlockEvictionOnRegionMovement.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java index d60c5e30dbc9..67e2992700ed 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.SingleProcessHBaseCluster; import org.apache.hadoop.hbase.StartTestingClusterOption; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Put; @@ -38,6 +39,7 @@ import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.io.hfile.bucket.BucketCache; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.IOTests; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -99,10 +101,15 @@ public void testBlockEvictionOnRegionMove() throws Exception { ? cluster.getRegionServer(1) : cluster.getRegionServer(0); assertTrue(regionServingRS.getBlockCache().isPresent()); + + //wait for running prefetch threads to be completed. + Waiter.waitFor(this.conf, 200, () -> PrefetchExecutor.getPrefetchFutures().isEmpty()); + long oldUsedCacheSize = regionServingRS.getBlockCache().get().getBlockCaches()[1].getCurrentSize(); assertNotEquals(0, oldUsedCacheSize); + Admin admin = TEST_UTIL.getAdmin(); RegionInfo regionToMove = regionServingRS.getRegions(tableRegionMove).get(0).getRegionInfo(); admin.move(regionToMove.getEncodedNameAsBytes(), From 25beeca5ff9e40b409e508a8c115d90b7faf67a0 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Tue, 22 Apr 2025 10:41:30 +0100 Subject: [PATCH 5/6] spotless Change-Id: Ia7d67f5593aaaefe591e438c00036c2846dc7f8f --- .../hbase/io/hfile/TestBlockEvictionOnRegionMovement.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java index 67e2992700ed..d1e6d3125876 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java @@ -39,7 +39,6 @@ import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; -import org.apache.hadoop.hbase.io.hfile.bucket.BucketCache; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.IOTests; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -102,14 +101,13 @@ public void testBlockEvictionOnRegionMove() throws Exception { : cluster.getRegionServer(0); assertTrue(regionServingRS.getBlockCache().isPresent()); - //wait for running prefetch threads to be completed. + // wait for running prefetch threads to be completed. Waiter.waitFor(this.conf, 200, () -> PrefetchExecutor.getPrefetchFutures().isEmpty()); long oldUsedCacheSize = regionServingRS.getBlockCache().get().getBlockCaches()[1].getCurrentSize(); assertNotEquals(0, oldUsedCacheSize); - Admin admin = TEST_UTIL.getAdmin(); RegionInfo regionToMove = regionServingRS.getRegions(tableRegionMove).get(0).getRegionInfo(); admin.move(regionToMove.getEncodedNameAsBytes(), From 660b17b2a6db5eb38a34ab5a9ebdcf51b04e5838 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Fri, 25 Apr 2025 14:27:00 +0100 Subject: [PATCH 6/6] added javadoc comments Change-Id: Id6390798d91d6f557caa0ed0007eda817c4ac16c --- .../hbase/io/hfile/bucket/BucketCache.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java index e4ff80b1b3f4..fff3f1dec67b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java @@ -900,6 +900,22 @@ boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey, BucketEnt return false; } + /** + * Since HBASE-29249, the following properties governin freeSpace behaviour and block priorities + * were made dynamically configurable: - hbase.bucketcache.acceptfactor - + * hbase.bucketcache.minfactor - hbase.bucketcache.extrafreefactor - + * hbase.bucketcache.single.factor - hbase.bucketcache.multi.factor - + * hbase.bucketcache.multi.factor - hbase.bucketcache.memory.factor The + * hbase.bucketcache.queue.addition.waittime property allows for introducing a delay in the + * publishing of blocks for the cache writer threads during prefetch reads only (client reads + * wouldn't get delayed). It has also been made dynamic configurable since HBASE-29249. The + * hbase.bucketcache.persist.intervalinmillis propperty determines the frequency for saving the + * persistent cache, and it has also been made dynamically configurable since HBASE-29249. The + * hbase.bucketcache.persistence.chunksize property determines the size of the persistent file + * splits (due to the limitation of maximum allowed protobuff size), and it has also been made + * dynamically configurable since HBASE-29249. + * @param config the new configuration to be updated. + */ @Override public void onConfigurationChange(Configuration config) { this.acceptableFactor = conf.getFloat(ACCEPT_FACTOR_CONFIG_NAME, DEFAULT_ACCEPT_FACTOR);