From 16aaa484e1eff4b670ca1c5ab3a2082878ff65d2 Mon Sep 17 00:00:00 2001 From: jinhyukify Date: Sun, 26 Apr 2026 23:15:29 +0900 Subject: [PATCH 1/4] HBASE-30117 Fix HBase shell CONFIGURATION for column family using setValue instead of setConfiguration --- hbase-shell/src/main/ruby/hbase/admin.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb index 2b3b1086f75e..3b70b41db3f2 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/hbase-shell/src/main/ruby/hbase/admin.rb @@ -1294,7 +1294,13 @@ def cfd(arg, tdb) end set_user_metadata(cfdb, arg.delete(METADATA)) if arg[METADATA] - set_descriptor_config(cfdb, arg.delete(CONFIGURATION)) if arg[CONFIGURATION] + if arg[CONFIGURATION] + raise(ArgumentError, "#{CONFIGURATION} must be a Hash type") unless arg[CONFIGURATION].is_a?(Hash) + for k, v in arg.delete(CONFIGURATION) + v = v.to_s unless v.nil? + cfdb.setConfiguration(k, v) + end + end if arg.include?(ColumnFamilyDescriptorBuilder::DFS_REPLICATION) cfdb.setDFSReplication(JInteger.valueOf(arg.delete(ColumnFamilyDescriptorBuilder::DFS_REPLICATION))) end From bc5b64c99bb426ef16095cc1660e0646a08e07bc Mon Sep 17 00:00:00 2001 From: jinhyukify Date: Mon, 4 May 2026 16:59:20 +0900 Subject: [PATCH 2/4] Revert "HBASE-30117 Fix HBase shell CONFIGURATION for column family using setValue instead of setConfiguration" This reverts commit 16aaa484e1eff4b670ca1c5ab3a2082878ff65d2. --- hbase-shell/src/main/ruby/hbase/admin.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb index 3b70b41db3f2..2b3b1086f75e 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/hbase-shell/src/main/ruby/hbase/admin.rb @@ -1294,13 +1294,7 @@ def cfd(arg, tdb) end set_user_metadata(cfdb, arg.delete(METADATA)) if arg[METADATA] - if arg[CONFIGURATION] - raise(ArgumentError, "#{CONFIGURATION} must be a Hash type") unless arg[CONFIGURATION].is_a?(Hash) - for k, v in arg.delete(CONFIGURATION) - v = v.to_s unless v.nil? - cfdb.setConfiguration(k, v) - end - end + set_descriptor_config(cfdb, arg.delete(CONFIGURATION)) if arg[CONFIGURATION] if arg.include?(ColumnFamilyDescriptorBuilder::DFS_REPLICATION) cfdb.setDFSReplication(JInteger.valueOf(arg.delete(ColumnFamilyDescriptorBuilder::DFS_REPLICATION))) end From 1654f496ad4e6319eb5bae817fb13289cec89b84 Mon Sep 17 00:00:00 2001 From: jinhyukify Date: Mon, 4 May 2026 18:47:19 +0900 Subject: [PATCH 3/4] HBASE-30117 TableDescriptorChecker should verify CF date-tiered and bloom filter configuration set via setValue --- .../hbase/util/TableDescriptorChecker.java | 7 +-- .../client/TestIllegalTableDescriptor.java | 48 ++++++++++++------- .../util/TestTableDescriptorChecker.java | 35 ++++++++++++++ 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/TableDescriptorChecker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/TableDescriptorChecker.java index 409cc284dee2..691103ba256f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/TableDescriptorChecker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/TableDescriptorChecker.java @@ -233,8 +233,8 @@ private static void checkDateTieredCompactionForTimeRangeDataTiering(final Confi checkDateTieredCompactionForTimeRangeDataTiering(conf); for (ColumnFamilyDescriptor cfd : td.getColumnFamilies()) { // Column family level configurations - Configuration cfdConf = - new CompoundConfiguration().add(conf).addStringMap(cfd.getConfiguration()); + Configuration cfdConf = new CompoundConfiguration().add(conf) + .addStringMap(cfd.getConfiguration()).addBytesMap(cfd.getValues()); checkDateTieredCompactionForTimeRangeDataTiering(cfdConf); } } @@ -318,7 +318,8 @@ private static void checkBloomFilterType(final Configuration conf, final TableDe throws IOException { warnOrThrowExceptionForFailure(conf, () -> { for (ColumnFamilyDescriptor cfd : td.getColumnFamilies()) { - Configuration cfdConf = new CompoundConfiguration().addStringMap(cfd.getConfiguration()); + Configuration cfdConf = new CompoundConfiguration().addStringMap(cfd.getConfiguration()) + .addBytesMap(cfd.getValues()); try { BloomFilterUtil.getBloomFilterParam(cfd.getBloomFilterType(), cfdConf); } catch (IllegalArgumentException e) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java index 666cfbdf54a9..bc6fc8ae2b65 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java @@ -212,24 +212,38 @@ public void testIllegalTableDescriptorWithDataTiering() throws IOException { "org.apache.hadoop.hbase.regionserver.DefaultStoreEngine"); checkTableIsIllegal(builder.build()); - // column family level configuration changes - builder = TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())); - cfBuilder = ColumnFamilyDescriptorBuilder.newBuilder(FAMILY); - - // First scenario: DataTieringType set to TIME_RANGE without DateTieredStoreEngine - cfBuilder.setConfiguration(DataTieringManager.DATATIERING_KEY, - DataTieringType.TIME_RANGE.name()); - checkTableIsIllegal(builder.setColumnFamily(cfBuilder.build()).build()); - - // Second scenario: DataTieringType set to TIME_RANGE with DateTieredStoreEngine - cfBuilder.setConfiguration(StoreEngine.STORE_ENGINE_CLASS_KEY, - "org.apache.hadoop.hbase.regionserver.DateTieredStoreEngine"); - checkTableIsLegal(builder.modifyColumnFamily(cfBuilder.build()).build()); + // column family level configuration changes — exercised via both + // ColumnFamilyDescriptorBuilder#setConfiguration (configuration map) and + // ColumnFamilyDescriptorBuilder#setValue (values map) so that + // TableDescriptorChecker is exercised against both underlying storage maps. + for (boolean viaSetValue : new boolean[] { false, true }) { + builder = TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())); + cfBuilder = ColumnFamilyDescriptorBuilder.newBuilder(FAMILY); + + // First scenario: DataTieringType set to TIME_RANGE without DateTieredStoreEngine + setCfKey(cfBuilder, viaSetValue, DataTieringManager.DATATIERING_KEY, + DataTieringType.TIME_RANGE.name()); + checkTableIsIllegal(builder.setColumnFamily(cfBuilder.build()).build()); + + // Second scenario: DataTieringType set to TIME_RANGE with DateTieredStoreEngine + setCfKey(cfBuilder, viaSetValue, StoreEngine.STORE_ENGINE_CLASS_KEY, + "org.apache.hadoop.hbase.regionserver.DateTieredStoreEngine"); + checkTableIsLegal(builder.modifyColumnFamily(cfBuilder.build()).build()); + + // Third scenario: Disabling DateTieredStoreEngine while Time Range DataTiering is active + setCfKey(cfBuilder, viaSetValue, StoreEngine.STORE_ENGINE_CLASS_KEY, + "org.apache.hadoop.hbase.regionserver.DefaultStoreEngine"); + checkTableIsIllegal(builder.modifyColumnFamily(cfBuilder.build()).build()); + } + } - // Third scenario: Disabling DateTieredStoreEngine while Time Range DataTiering is active - cfBuilder.setConfiguration(StoreEngine.STORE_ENGINE_CLASS_KEY, - "org.apache.hadoop.hbase.regionserver.DefaultStoreEngine"); - checkTableIsIllegal(builder.modifyColumnFamily(cfBuilder.build()).build()); + private static void setCfKey(ColumnFamilyDescriptorBuilder cfb, boolean viaSetValue, String key, + String value) { + if (viaSetValue) { + cfb.setValue(key, value); + } else { + cfb.setConfiguration(key, value); + } } private void checkTableIsLegal(TableDescriptor tableDescriptor) throws IOException { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestTableDescriptorChecker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestTableDescriptorChecker.java index fcc2ab4ce8fd..17a36b43750d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestTableDescriptorChecker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestTableDescriptorChecker.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.conf.ConfigKey; +import org.apache.hadoop.hbase.regionserver.BloomType; import org.apache.hadoop.hbase.testclassification.MiscTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.jupiter.api.Tag; @@ -84,4 +85,38 @@ public void testSanityCheck() throws IOException { TableDescriptorChecker.sanityCheck(conf, t.build()); } } + + @Test + public void testBloomFilterPrefixLengthValidation() throws IOException { + Configuration conf = new Configuration(); + String key = BloomFilterUtil.PREFIX_LENGTH_KEY; + + for (boolean viaSetValue : new boolean[] { true, false }) { + ColumnFamilyDescriptorBuilder cf = ColumnFamilyDescriptorBuilder.newBuilder("cf".getBytes()) + .setBloomFilterType(BloomType.ROWPREFIX_FIXED_LENGTH); + TableDescriptorBuilder t = TableDescriptorBuilder.newBuilder(TableName.valueOf("test")); + + // Invalid: prefix length must be > 0 for ROWPREFIX_FIXED_LENGTH + if (viaSetValue) { + cf.setValue(key, "0"); + } else { + cf.setConfiguration(key, "0"); + } + t.setColumnFamily(cf.build()); + assertThrows(DoNotRetryIOException.class, + () -> TableDescriptorChecker.sanityCheck(conf, t.build()), + "Should reject ROWPREFIX_FIXED_LENGTH with prefix length 0 set via " + + (viaSetValue ? "setValue" : "setConfiguration")); + + // Fix the error. + if (viaSetValue) { + cf.setValue(key, "5"); + } else { + cf.setConfiguration(key, "5"); + } + t.removeColumnFamily("cf".getBytes()); + t.setColumnFamily(cf.build()); + TableDescriptorChecker.sanityCheck(conf, t.build()); + } + } } From aa472da41b0ce2d0f3ed8f7c90b64dd8eae5b7a2 Mon Sep 17 00:00:00 2001 From: jinhyukify Date: Wed, 6 May 2026 14:21:09 +0900 Subject: [PATCH 4/4] HBASE-30117 Simplify comment in TestIllegalTableDescriptor --- .../hadoop/hbase/client/TestIllegalTableDescriptor.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java index bc6fc8ae2b65..9b314ba87c57 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIllegalTableDescriptor.java @@ -212,10 +212,7 @@ public void testIllegalTableDescriptorWithDataTiering() throws IOException { "org.apache.hadoop.hbase.regionserver.DefaultStoreEngine"); checkTableIsIllegal(builder.build()); - // column family level configuration changes — exercised via both - // ColumnFamilyDescriptorBuilder#setConfiguration (configuration map) and - // ColumnFamilyDescriptorBuilder#setValue (values map) so that - // TableDescriptorChecker is exercised against both underlying storage maps. + // column family level configuration changes for (boolean viaSetValue : new boolean[] { false, true }) { builder = TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())); cfBuilder = ColumnFamilyDescriptorBuilder.newBuilder(FAMILY);