hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From li...@apache.org
Subject svn commit: r1532841 - in /hbase/branches/0.89-fb/src: main/java/org/apache/hadoop/hbase/regionserver/Store.java test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java
Date Wed, 16 Oct 2013 18:18:19 GMT
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));
   }
 }
 



Mime
View raw message