Author: liyin Date: Wed Oct 16 18:18:19 2013 New Revision: 1532841 URL: http://svn.apache.org/r1532841 Log: [master] Bug fix to ensure properties in the TableDescriptor don't get overriden during Online Config Change
Author: gauravm Summary: During testing, I found that certain properties such as the compaction ratio, which were set in the Table Descriptor, were getting overriden when we were trying to do an online configuration change. This diff fixes that. Test Plan: Improved the TestRegionServerOnlineConfigChange test to cover this case. Reviewers: aaiyer, adela, manukranthk, rshroff, fan Reviewed By: fan CC: hbase-eng@ Differential Revision: https://phabricator.fb.com/D1011438 Task ID: 2842041 Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1532841&r1=1532840&r2=1532841&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original) +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Wed Oct 16 18:18:19 2013 @@ -2023,11 +2023,15 @@ public class Store extends SchemaConfigu } @Override - public void notifyOnChange(Configuration conf) { + public void notifyOnChange(Configuration confParam) { + // Re-create the CompoundConfiguration in the manner that it is created in + // the hierarchy. Add the conf first, then add the values from the table + // descriptor, and then the column family descriptor. this.conf = new CompoundConfiguration() - .add(conf) + .add(confParam) + .add(getHRegionInfo().getTableDesc().getValues()) .add(family.getValues()); - compactionManager.updateConfiguration(conf); + compactionManager.updateConfiguration(this.conf); } } Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java?rev=1532841&r1=1532840&r2=1532841&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java (original) +++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java Wed Oct 16 18:18:19 2013 @@ -62,6 +62,9 @@ public class TestRegionServerOnlineConfi final String prop = "prop"; final String cf2PropVal = "customVal"; final String newGeneralPropVal = "newGeneralVal"; + final String tableDescProp = "tableDescProp"; + final String tableDescPropCustomVal = "tableDescPropCustomVal"; + final String tableDescPropGeneralVal = "tableDescPropGeneralVal"; @Override @@ -258,6 +261,7 @@ public class TestRegionServerOnlineConfi private HTable createTableWithPerCFConfigurations() throws IOException { byte[] tableName = Bytes.toBytesBinary(table2Str); HTableDescriptor desc = new HTableDescriptor(tableName); + for (byte[] family : FAMILIES) { HColumnDescriptor hcd = new HColumnDescriptor(family); if (Bytes.equals(family, COLUMN_FAMILY2)) { @@ -266,6 +270,11 @@ public class TestRegionServerOnlineConfi } desc.addFamily(hcd); } + + // Also try setting a property in the Table Descriptor + desc.setValue(Bytes.toBytes(tableDescProp), + Bytes.toBytes(tableDescPropCustomVal)); + (new HBaseAdmin(conf)).createTable(desc); return new HTable(conf, tableName); } @@ -293,6 +302,10 @@ public class TestRegionServerOnlineConfi // Set the value of prop to some other value in the conf. conf.set(prop, newGeneralPropVal); + // Add a general value for the tableDescProp. But we shouldn't see this + // value in the HTable's store instances, since we are explicitly + // specifying this property through the Table Descriptor. + conf.set(tableDescProp, tableDescPropGeneralVal); // Simulate an online config change HRegionServer.configurationManager.notifyAllObservers(conf); @@ -309,6 +322,11 @@ public class TestRegionServerOnlineConfi // However, the value for prop in CF2 shouldn't change since we explicitly // specified a value for that property in the HCD. assertEquals(cf2PropVal, s2.conf.get(prop)); + + // Here we show that the properties which are set in the table descriptor + // don't get overriden either. + assertEquals(tableDescPropCustomVal, s1.conf.get(tableDescProp)); + assertEquals(tableDescPropCustomVal, s2.conf.get(tableDescProp)); } }
