From 397377b27037aacc644546c6060a5fc0588fbe96 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Tue, 23 Sep 2025 10:50:25 +0100 Subject: [PATCH 1/2] HBASE-29623 Blocks for CFs with BlockCache disabled may still get cached on write or compaction Change-Id: I3c184910d815b7855f27086937e4f74307aeac7b --- .../hadoop/hbase/io/hfile/CacheConfig.java | 69 ++++++++++--------- .../hbase/io/hfile/TestCacheConfig.java | 21 ++++-- 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java index 72ca37c0557c..ce54d9ce98f4 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 @@ -132,7 +132,7 @@ public class CacheConfig implements PropagatingConfigurationObserver { private volatile boolean cacheDataOnRead; /** Whether blocks should be flagged as in-memory when being cached */ - private final boolean inMemory; + private boolean inMemory; /** Whether data blocks should be cached when new files are written */ private volatile boolean cacheDataOnWrite; @@ -147,29 +147,29 @@ public class CacheConfig implements PropagatingConfigurationObserver { private volatile boolean evictOnClose; /** Whether data blocks should be stored in compressed and/or encrypted form in the cache */ - private final boolean cacheDataCompressed; + private boolean cacheDataCompressed; /** Whether data blocks should be prefetched into the cache */ - private final boolean prefetchOnOpen; + private boolean prefetchOnOpen; /** * Whether data blocks should be cached when compacted file is written */ - private final boolean cacheCompactedDataOnWrite; + private boolean cacheCompactedDataOnWrite; /** * Determine threshold beyond which we do not cache blocks on compaction */ private long cacheCompactedDataOnWriteThreshold; - private final boolean dropBehindCompaction; + private boolean dropBehindCompaction; // Local reference to the block cache private final BlockCache blockCache; private final ByteBuffAllocator byteBuffAllocator; - private final double heapUsageThreshold; + private double heapUsageThreshold; /** * Create a cache configuration using the specified configuration object and defaults for family @@ -191,32 +191,37 @@ public CacheConfig(Configuration conf, BlockCache blockCache) { */ public CacheConfig(Configuration conf, ColumnFamilyDescriptor family, BlockCache blockCache, ByteBuffAllocator byteBuffAllocator) { - this.cacheDataOnRead = conf.getBoolean(CACHE_DATA_ON_READ_KEY, DEFAULT_CACHE_DATA_ON_READ) - && (family == null ? true : family.isBlockCacheEnabled()); - this.inMemory = family == null ? DEFAULT_IN_MEMORY : family.isInMemory(); - this.cacheDataCompressed = - conf.getBoolean(CACHE_DATA_BLOCKS_COMPRESSED_KEY, DEFAULT_CACHE_DATA_COMPRESSED); - this.dropBehindCompaction = - conf.getBoolean(DROP_BEHIND_CACHE_COMPACTION_KEY, DROP_BEHIND_CACHE_COMPACTION_DEFAULT); - // For the following flags we enable them regardless of per-schema settings - // if they are enabled in the global configuration. - this.cacheDataOnWrite = conf.getBoolean(CACHE_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_DATA_ON_WRITE) - || (family == null ? false : family.isCacheDataOnWrite()); - this.cacheIndexesOnWrite = - conf.getBoolean(CACHE_INDEX_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_INDEXES_ON_WRITE) - || (family == null ? false : family.isCacheIndexesOnWrite()); - this.cacheBloomsOnWrite = - conf.getBoolean(CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_BLOOMS_ON_WRITE) - || (family == null ? false : family.isCacheBloomsOnWrite()); - this.evictOnClose = conf.getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE) - || (family == null ? false : family.isEvictBlocksOnClose()); - this.prefetchOnOpen = conf.getBoolean(PREFETCH_BLOCKS_ON_OPEN_KEY, DEFAULT_PREFETCH_ON_OPEN) - || (family == null ? false : family.isPrefetchBlocksOnOpen()); - this.cacheCompactedDataOnWrite = - conf.getBoolean(CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE); - this.cacheCompactedDataOnWriteThreshold = getCacheCompactedBlocksOnWriteThreshold(conf); - this.heapUsageThreshold = - conf.getDouble(PREFETCH_HEAP_USAGE_THRESHOLD, DEFAULT_PREFETCH_HEAP_USAGE_THRESHOLD); + if (family==null || family.isBlockCacheEnabled()) { + this.cacheDataOnRead = conf.getBoolean(CACHE_DATA_ON_READ_KEY, DEFAULT_CACHE_DATA_ON_READ); + this.inMemory = family == null ? DEFAULT_IN_MEMORY : family.isInMemory(); + this.cacheDataCompressed = conf.getBoolean(CACHE_DATA_BLOCKS_COMPRESSED_KEY, DEFAULT_CACHE_DATA_COMPRESSED); + this.dropBehindCompaction = conf.getBoolean(DROP_BEHIND_CACHE_COMPACTION_KEY, DROP_BEHIND_CACHE_COMPACTION_DEFAULT); + // For the following flags we enable them regardless of per-schema settings + // if they are enabled in the global configuration. + this.cacheDataOnWrite = + conf.getBoolean(CACHE_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_DATA_ON_WRITE) || (family == null ? + false : + family.isCacheDataOnWrite()); + this.cacheIndexesOnWrite = + conf.getBoolean(CACHE_INDEX_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_INDEXES_ON_WRITE) || ( + family == null ? false : family.isCacheIndexesOnWrite()); + this.cacheBloomsOnWrite = + conf.getBoolean(CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_BLOOMS_ON_WRITE) || ( + family == null ? false : family.isCacheBloomsOnWrite()); + this.evictOnClose = conf.getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE) || ( + family == null ? + false : + family.isEvictBlocksOnClose()); + this.prefetchOnOpen = + conf.getBoolean(PREFETCH_BLOCKS_ON_OPEN_KEY, DEFAULT_PREFETCH_ON_OPEN) || (family == null ? + false : + family.isPrefetchBlocksOnOpen()); + this.cacheCompactedDataOnWrite = + conf.getBoolean(CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE); + this.cacheCompactedDataOnWriteThreshold = getCacheCompactedBlocksOnWriteThreshold(conf); + this.heapUsageThreshold = + conf.getDouble(PREFETCH_HEAP_USAGE_THRESHOLD, DEFAULT_PREFETCH_HEAP_USAGE_THRESHOLD); + } this.blockCache = blockCache; this.byteBuffAllocator = byteBuffAllocator; } 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 f03d7fb2c016..0222938e485d 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 @@ -190,12 +190,14 @@ void basicBlockCacheOps(final BlockCache bc, final CacheConfig cc, final boolean @Test public void testDisableCacheDataBlock() throws IOException { + //First tests the default configs behaviour and block cache enabled Configuration conf = HBaseConfiguration.create(); CacheConfig cacheConfig = new CacheConfig(conf); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.DATA)); assertFalse(cacheConfig.shouldCacheCompressed(BlockCategory.DATA)); assertFalse(cacheConfig.shouldCacheDataCompressed()); assertFalse(cacheConfig.shouldCacheDataOnWrite()); + assertFalse(cacheConfig.shouldCacheCompactedBlocksOnWrite()); assertTrue(cacheConfig.shouldCacheDataOnRead()); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.INDEX)); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.META)); @@ -203,10 +205,12 @@ public void testDisableCacheDataBlock() throws IOException { assertFalse(cacheConfig.shouldCacheBloomsOnWrite()); assertFalse(cacheConfig.shouldCacheIndexesOnWrite()); + //Tests block cache enabled and related cache on write flags enabled conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, true); conf.setBoolean(CacheConfig.CACHE_DATA_BLOCKS_COMPRESSED_KEY, true); conf.setBoolean(CacheConfig.CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, true); conf.setBoolean(CacheConfig.CACHE_INDEX_BLOCKS_ON_WRITE_KEY, true); + conf.setBoolean(CacheConfig.CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, true); cacheConfig = new CacheConfig(conf); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.DATA)); @@ -219,9 +223,12 @@ public void testDisableCacheDataBlock() throws IOException { assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.BLOOM)); assertTrue(cacheConfig.shouldCacheBloomsOnWrite()); assertTrue(cacheConfig.shouldCacheIndexesOnWrite()); + assertTrue(cacheConfig.shouldCacheCompactedBlocksOnWrite()); + //Tests block cache enabled but related cache on read/write properties disabled conf.setBoolean(CacheConfig.CACHE_DATA_ON_READ_KEY, false); conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, false); + conf.setBoolean(CacheConfig.CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, false); cacheConfig = new CacheConfig(conf); assertFalse(cacheConfig.shouldCacheBlockOnRead(BlockCategory.DATA)); @@ -229,14 +236,20 @@ public void testDisableCacheDataBlock() throws IOException { assertFalse(cacheConfig.shouldCacheDataCompressed()); assertFalse(cacheConfig.shouldCacheDataOnWrite()); assertFalse(cacheConfig.shouldCacheDataOnRead()); + assertFalse(cacheConfig.shouldCacheCompactedBlocksOnWrite()); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.INDEX)); assertFalse(cacheConfig.shouldCacheBlockOnRead(BlockCategory.META)); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.BLOOM)); assertTrue(cacheConfig.shouldCacheBloomsOnWrite()); assertTrue(cacheConfig.shouldCacheIndexesOnWrite()); - conf.setBoolean(CacheConfig.CACHE_DATA_ON_READ_KEY, true); - conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, false); + //Finally tests block cache disabled in the column family but all cache on read/write + // properties enabled in the config. + conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, true); + conf.setBoolean(CacheConfig.CACHE_DATA_BLOCKS_COMPRESSED_KEY, true); + conf.setBoolean(CacheConfig.CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, true); + conf.setBoolean(CacheConfig.CACHE_INDEX_BLOCKS_ON_WRITE_KEY, true); + conf.setBoolean(CacheConfig.CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, true); ColumnFamilyDescriptor columnFamilyDescriptor = ColumnFamilyDescriptorBuilder .newBuilder(Bytes.toBytes("testDisableCacheDataBlock")).setBlockCacheEnabled(false).build(); @@ -250,8 +263,8 @@ public void testDisableCacheDataBlock() throws IOException { assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.INDEX)); assertFalse(cacheConfig.shouldCacheBlockOnRead(BlockCategory.META)); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.BLOOM)); - assertTrue(cacheConfig.shouldCacheBloomsOnWrite()); - assertTrue(cacheConfig.shouldCacheIndexesOnWrite()); + assertFalse(cacheConfig.shouldCacheBloomsOnWrite()); + assertFalse(cacheConfig.shouldCacheIndexesOnWrite()); } @Test From 08d9a15e174a111a4250f7a6a534679d780ffebd Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Wed, 24 Sep 2025 13:45:15 +0100 Subject: [PATCH 2/2] spotless Change-Id: I700895e55468e2a608cf4890cd17cc091a3b03c0 --- .../hadoop/hbase/io/hfile/CacheConfig.java | 37 +++++++++---------- .../hbase/io/hfile/TestCacheConfig.java | 8 ++-- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java index ce54d9ce98f4..fc8f4d569176 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 @@ -191,33 +191,30 @@ public CacheConfig(Configuration conf, BlockCache blockCache) { */ public CacheConfig(Configuration conf, ColumnFamilyDescriptor family, BlockCache blockCache, ByteBuffAllocator byteBuffAllocator) { - if (family==null || family.isBlockCacheEnabled()) { + if (family == null || family.isBlockCacheEnabled()) { this.cacheDataOnRead = conf.getBoolean(CACHE_DATA_ON_READ_KEY, DEFAULT_CACHE_DATA_ON_READ); this.inMemory = family == null ? DEFAULT_IN_MEMORY : family.isInMemory(); - this.cacheDataCompressed = conf.getBoolean(CACHE_DATA_BLOCKS_COMPRESSED_KEY, DEFAULT_CACHE_DATA_COMPRESSED); - this.dropBehindCompaction = conf.getBoolean(DROP_BEHIND_CACHE_COMPACTION_KEY, DROP_BEHIND_CACHE_COMPACTION_DEFAULT); + this.cacheDataCompressed = + conf.getBoolean(CACHE_DATA_BLOCKS_COMPRESSED_KEY, DEFAULT_CACHE_DATA_COMPRESSED); + this.dropBehindCompaction = + conf.getBoolean(DROP_BEHIND_CACHE_COMPACTION_KEY, DROP_BEHIND_CACHE_COMPACTION_DEFAULT); // For the following flags we enable them regardless of per-schema settings // if they are enabled in the global configuration. this.cacheDataOnWrite = - conf.getBoolean(CACHE_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_DATA_ON_WRITE) || (family == null ? - false : - family.isCacheDataOnWrite()); + conf.getBoolean(CACHE_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_DATA_ON_WRITE) + || (family == null ? false : family.isCacheDataOnWrite()); this.cacheIndexesOnWrite = - conf.getBoolean(CACHE_INDEX_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_INDEXES_ON_WRITE) || ( - family == null ? false : family.isCacheIndexesOnWrite()); + conf.getBoolean(CACHE_INDEX_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_INDEXES_ON_WRITE) + || (family == null ? false : family.isCacheIndexesOnWrite()); this.cacheBloomsOnWrite = - conf.getBoolean(CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_BLOOMS_ON_WRITE) || ( - family == null ? false : family.isCacheBloomsOnWrite()); - this.evictOnClose = conf.getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE) || ( - family == null ? - false : - family.isEvictBlocksOnClose()); - this.prefetchOnOpen = - conf.getBoolean(PREFETCH_BLOCKS_ON_OPEN_KEY, DEFAULT_PREFETCH_ON_OPEN) || (family == null ? - false : - family.isPrefetchBlocksOnOpen()); - this.cacheCompactedDataOnWrite = - conf.getBoolean(CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE); + conf.getBoolean(CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, DEFAULT_CACHE_BLOOMS_ON_WRITE) + || (family == null ? false : family.isCacheBloomsOnWrite()); + this.evictOnClose = conf.getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE) + || (family == null ? false : family.isEvictBlocksOnClose()); + this.prefetchOnOpen = conf.getBoolean(PREFETCH_BLOCKS_ON_OPEN_KEY, DEFAULT_PREFETCH_ON_OPEN) + || (family == null ? false : family.isPrefetchBlocksOnOpen()); + this.cacheCompactedDataOnWrite = conf.getBoolean(CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, + DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE); this.cacheCompactedDataOnWriteThreshold = getCacheCompactedBlocksOnWriteThreshold(conf); this.heapUsageThreshold = conf.getDouble(PREFETCH_HEAP_USAGE_THRESHOLD, DEFAULT_PREFETCH_HEAP_USAGE_THRESHOLD); 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 0222938e485d..50c0b717096a 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 @@ -190,7 +190,7 @@ void basicBlockCacheOps(final BlockCache bc, final CacheConfig cc, final boolean @Test public void testDisableCacheDataBlock() throws IOException { - //First tests the default configs behaviour and block cache enabled + // First tests the default configs behaviour and block cache enabled Configuration conf = HBaseConfiguration.create(); CacheConfig cacheConfig = new CacheConfig(conf); assertTrue(cacheConfig.shouldCacheBlockOnRead(BlockCategory.DATA)); @@ -205,7 +205,7 @@ public void testDisableCacheDataBlock() throws IOException { assertFalse(cacheConfig.shouldCacheBloomsOnWrite()); assertFalse(cacheConfig.shouldCacheIndexesOnWrite()); - //Tests block cache enabled and related cache on write flags enabled + // Tests block cache enabled and related cache on write flags enabled conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, true); conf.setBoolean(CacheConfig.CACHE_DATA_BLOCKS_COMPRESSED_KEY, true); conf.setBoolean(CacheConfig.CACHE_BLOOM_BLOCKS_ON_WRITE_KEY, true); @@ -225,7 +225,7 @@ public void testDisableCacheDataBlock() throws IOException { assertTrue(cacheConfig.shouldCacheIndexesOnWrite()); assertTrue(cacheConfig.shouldCacheCompactedBlocksOnWrite()); - //Tests block cache enabled but related cache on read/write properties disabled + // Tests block cache enabled but related cache on read/write properties disabled conf.setBoolean(CacheConfig.CACHE_DATA_ON_READ_KEY, false); conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, false); conf.setBoolean(CacheConfig.CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY, false); @@ -243,7 +243,7 @@ public void testDisableCacheDataBlock() throws IOException { assertTrue(cacheConfig.shouldCacheBloomsOnWrite()); assertTrue(cacheConfig.shouldCacheIndexesOnWrite()); - //Finally tests block cache disabled in the column family but all cache on read/write + // Finally tests block cache disabled in the column family but all cache on read/write // properties enabled in the config. conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, true); conf.setBoolean(CacheConfig.CACHE_DATA_BLOCKS_COMPRESSED_KEY, true);