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..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 @@ -213,23 +213,34 @@ public void testIllegalTableDescriptorWithDataTiering() throws IOException { 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()); + 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()); + } + } }