hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From te...@apache.org
Subject svn commit: r1535442 - in /hbase/branches/0.94/src: main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java
Date Thu, 24 Oct 2013 17:07:55 GMT
Author: tedyu
Date: Thu Oct 24 17:07:55 2013
New Revision: 1535442

URL: http://svn.apache.org/r1535442
Log:
HBASE-9819 Backport HBASE-8372 'Provide mutability to CompoundConfiguration' to 0.94 (Ted
Yu)


Modified:
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
    hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java?rev=1535442&r1=1535441&r2=1535442&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
(original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
Thu Oct 24 17:07:55 2013
@@ -19,12 +19,10 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
-import java.io.DataInput;
 import java.io.DataOutput;
-import java.io.IOException;
+import java.io.IOException; 
 import java.io.OutputStream;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -36,7 +34,6 @@ import org.apache.hadoop.classification.
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.hadoop.util.StringUtils;
 
 /**
  * Do a shallow merge of multiple KV configuration pools. This is a very useful
@@ -53,9 +50,15 @@ import org.apache.hadoop.util.StringUtil
  * This class is package private because we expect significant refactoring here
  * on the HBase side when certain HDFS changes are added & ubiquitous. Will
  * revisit expanding access at that point.
+ *
+ * WARNING: The values set in the CompoundConfiguration are do not handle Property variable
+ * substitution.  However, if they are set in the underlying configuration substitutions
are
+ * done.
  */
 @InterfaceAudience.Private
 class CompoundConfiguration extends Configuration {
+	private Configuration mutableConf = null;
+
   /**
    * Default Constructor. Initializes empty configuration
    */
@@ -78,51 +81,73 @@ class CompoundConfiguration extends Conf
    * These initial APIs actually required original thought
    ***************************************************************************/
 
+  static class ImmutableConfWrapper implements  ImmutableConfigMap {
+    Configuration c;
+    
+    ImmutableConfWrapper(Configuration conf) {
+      c = conf;
+    }
+
+    @Override
+    public Iterator<Map.Entry<String,String>> iterator() {
+      return c.iterator();
+    }
+    
+    @Override
+    public String get(String key) {
+      return c.get(key);
+    }
+
+    @Override
+    public String getRaw(String key) {
+      return c.getRaw(key);
+    }
+
+    @Override
+    public Class<?> getClassByName(String name)
+        throws ClassNotFoundException {
+      return c.getClassByName(name);
+    }
+
+    @Override
+    public int size() {
+      return c.size();
+    }
+
+    @Override
+    public String toString() {
+      return c.toString();
+    }
+  }
+
+  /**
+   * If set has been called, it will create a mutableConf.  This converts the mutableConf
to an
+   * immutable one and resets it to allow a new mutable conf.  This is used when a new map
or
+   * conf is added to the compound configuration to preserve proper override semantics.
+   */
+  void freezeMutableConf() {
+    if (mutableConf == null) {
+      // do nothing if there is no current mutableConf
+      return;
+    }
+
+    this.configs.add(0, new ImmutableConfWrapper(mutableConf));
+    mutableConf = null;
+  }
+
   /**
    * Add Hadoop Configuration object to config list
    * @param conf configuration object
    * @return this, for builder pattern
    */
   public CompoundConfiguration add(final Configuration conf) {
+    freezeMutableConf();
     if (conf instanceof CompoundConfiguration) {
       this.configs.addAll(0, ((CompoundConfiguration) conf).configs);
       return this;
     }
     // put new config at the front of the list (top priority)
-    this.configs.add(0, new ImmutableConfigMap() {
-      Configuration c = conf;
-
-      @Override
-      public String get(String key) {
-        return c.get(key);
-      }
-
-      @Override
-      public String getRaw(String key) {
-        return c.getRaw(key);
-      }
-
-      @Override
-      public Class<?> getClassByName(String name)
-          throws ClassNotFoundException {
-        return c.getClassByName(name);
-      }
-
-      @Override
-      public int size() {
-        return c.size();
-      }
-
-      @Override
-      public String toString() {
-        return c.toString();
-      }
-
-      @Override
-      public Iterator<Entry<String, String>> iterator() {
-        return c.iterator();
-      }
-    });
+    this.configs.add(0, new ImmutableConfWrapper(conf));
     return this;
   }
 
@@ -137,6 +162,8 @@ class CompoundConfiguration extends Conf
    */
   public CompoundConfiguration add(
       final Map<ImmutableBytesWritable, ImmutableBytesWritable> map) {
+    freezeMutableConf();
+
     // put new map at the front of the list (top priority)
     this.configs.add(0, new ImmutableConfigMap() {
       Map<ImmutableBytesWritable, ImmutableBytesWritable> m = map;
@@ -227,6 +254,54 @@ class CompoundConfiguration extends Conf
     return this;
   }
 
+  /**
+   * Add String map to config list. This map is generally created by HTableDescriptor
+   * or HColumnDescriptor, but can be abstractly used. The added configuration
+   * overrides the previous ones if there are name collisions.
+   *
+   * @return this, for builder pattern
+   */
+  public CompoundConfiguration addStringMap(final Map<String, String> map) {
+    freezeMutableConf();
+
+    // put new map at the front of the list (top priority)
+    this.configs.add(0, new ImmutableConfigMap() {
+      Map<String, String> m = map;
+
+      @Override
+      public Iterator<Map.Entry<String,String>> iterator() {
+        return map.entrySet().iterator();
+      }
+
+      @Override
+      public String get(String key) {
+        return m.get(key);
+      }
+
+      @Override
+      public String getRaw(String key) {
+        return get(key);
+      }
+
+      @Override
+      public Class<?> getClassByName(String name)
+      throws ClassNotFoundException {
+        return null;
+      }
+
+      @Override
+      public int size() {
+        return m.size();
+      }
+
+      @Override
+      public String toString() {
+        return m.toString();
+      }
+    });
+    return this;
+  }
+
   @Override
   public String toString() {
     StringBuffer sb = new StringBuffer();
@@ -239,6 +314,13 @@ class CompoundConfiguration extends Conf
 
   @Override
   public String get(String key) {
+    if (mutableConf != null) {
+      String value = mutableConf.get(key);
+      if (value != null) {
+        return value;
+      }
+    }
+
     for (ImmutableConfigMap m : this.configs) {
       String value = m.get(key);
       if (value != null) {
@@ -250,6 +332,13 @@ class CompoundConfiguration extends Conf
 
   @Override
   public String getRaw(String key) {
+    if (mutableConf != null) {
+      String value = mutableConf.getRaw(key);
+      if (value != null) {
+        return value;
+      }
+    }
+
     for (ImmutableConfigMap m : this.configs) {
       String value = m.getRaw(key);
       if (value != null) {
@@ -261,6 +350,13 @@ class CompoundConfiguration extends Conf
 
   @Override
   public Class<?> getClassByName(String name) throws ClassNotFoundException {
+    if (mutableConf != null) {
+      Class<?> value = mutableConf.getClassByName(name);
+      if (value != null) {
+        return value;
+      }
+    }
+
     for (ImmutableConfigMap m : this.configs) {
       try {
         Class<?> value = m.getClassByName(name);
@@ -275,9 +371,14 @@ class CompoundConfiguration extends Conf
     throw new ClassNotFoundException();
   }
 
+  // TODO: This method overestimates the number of configuration settings -- if a value is
masked
+  // by an overriding config or map, it will be counted multiple times.
   @Override
   public int size() {
     int ret = 0;
+    if (mutableConf != null) {
+      ret += mutableConf.size();
+    }
     for (ImmutableConfigMap m : this.configs) {
       ret += m.size();
     }
@@ -299,28 +400,36 @@ class CompoundConfiguration extends Conf
         }
       }
     }
+    // add mutations to this CompoundConfiguration last.
+    if (mutableConf != null) {
+      Iterator<Map.Entry<String, String>> miter = mutableConf.iterator();
+      while (miter.hasNext()) {
+        Map.Entry<String, String> entry = miter.next();
+        ret.put(entry.getKey(), entry.getValue());
+      }
+    }
 
     return UnmodifiableIterator.decorate(ret.entrySet().iterator());
   }
 
-  /***************************************************************************
-   * You should just ignore everything below this line unless there's a bug in
-   * Configuration.java...
+  /**
+   * Get the value of the <code>name</code>. If the key is deprecated,
+   * it returns the value of the first key which replaces the deprecated key
+   * and is not null.
+   * If no such property exists,
+   * then <code>defaultValue</code> is returned.
+
+   * The CompooundConfiguration does not do property substitution.  To do so we need
+   * Configuration.getProps to be protected or package visible.  Though in hadoop2 it is
+   * protected, in hadoop1 the method is private and not accessible.
    *
-   * Below get APIs are directly copied from Configuration.java Oh, how I wish
-   * this wasn't so! A tragically-sad example of why you use interfaces instead
-   * of inheritance.
+   * All of the get* methods call this overridden get method.
    *
-   * Why the duplication? We basically need to override Configuration.getProps
-   * or we'd need protected access to Configuration.properties so we can modify
-   * that pointer. There are a bunch of functions in the base Configuration that
-   * call getProps() and we need to use our derived version instead of the base
-   * version. We need to make a generic implementation that works across all
-   * HDFS versions. We should modify Configuration.properties in HDFS 1.0 to be
-   * protected, but we still need to have this code until that patch makes it to
-   * all the HDFS versions we support.
-   ***************************************************************************/
-
+   * @param name property name.
+   * @param defaultValue default value.
+   * @return property value, or <code>defaultValue</code> if the property
+   *         doesn't exist.
+   **/
   @Override
   public String get(String name, String defaultValue) {
     String ret = get(name);
@@ -328,200 +437,20 @@ class CompoundConfiguration extends Conf
   }
 
   @Override
-  public int getInt(String name, int defaultValue) {
-    String valueString = get(name);
-    if (valueString == null)
-      return defaultValue;
-    try {
-      String hexString = getHexDigits(valueString);
-      if (hexString != null) {
-        return Integer.parseInt(hexString, 16);
-      }
-      return Integer.parseInt(valueString);
-    } catch (NumberFormatException e) {
-      return defaultValue;
-    }
-  }
-
-  @Override
-  public long getLong(String name, long defaultValue) {
-    String valueString = get(name);
-    if (valueString == null)
-      return defaultValue;
-    try {
-      String hexString = getHexDigits(valueString);
-      if (hexString != null) {
-        return Long.parseLong(hexString, 16);
-      }
-      return Long.parseLong(valueString);
-    } catch (NumberFormatException e) {
-      return defaultValue;
-    }
-  }
-
-  protected String getHexDigits(String value) {
-    boolean negative = false;
-    String str = value;
-    String hexString = null;
-    if (value.startsWith("-")) {
-      negative = true;
-      str = value.substring(1);
-    }
-    if (str.startsWith("0x") || str.startsWith("0X")) {
-      hexString = str.substring(2);
-      if (negative) {
-        hexString = "-" + hexString;
-      }
-      return hexString;
-    }
-    return null;
-  }
-
-  @Override
-  public float getFloat(String name, float defaultValue) {
-    String valueString = get(name);
-    if (valueString == null)
-      return defaultValue;
-    try {
-      return Float.parseFloat(valueString);
-    } catch (NumberFormatException e) {
-      return defaultValue;
-    }
-  }
-
-  @Override
-  public boolean getBoolean(String name, boolean defaultValue) {
-    String valueString = get(name);
-    if ("true".equals(valueString))
-      return true;
-    else if ("false".equals(valueString))
-      return false;
-    else return defaultValue;
-  }
-
-  @Override
-  public IntegerRanges getRange(String name, String defaultValue) {
-    return new IntegerRanges(get(name, defaultValue));
-  }
-
-  @Override
-  public Collection<String> getStringCollection(String name) {
-    String valueString = get(name);
-    return StringUtils.getStringCollection(valueString);
-  }
-
-  @Override
-  public String[] getStrings(String name) {
-    String valueString = get(name);
-    return StringUtils.getStrings(valueString);
-  }
-
-  @Override
-  public String[] getStrings(String name, String... defaultValue) {
-    String valueString = get(name);
-    if (valueString == null) {
-      return defaultValue;
-    } else {
-      return StringUtils.getStrings(valueString);
-    }
-  }
-
-  @Override
-  public Class<?>[] getClasses(String name, Class<?>... defaultValue) {
-    String[] classnames = getStrings(name);
-    if (classnames == null)
-      return defaultValue;
-    try {
-      Class<?>[] classes = new Class<?>[classnames.length];
-      for (int i = 0; i < classnames.length; i++) {
-        classes[i] = getClassByName(classnames[i]);
-      }
-      return classes;
-    } catch (ClassNotFoundException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
-  @Override
-  public Class<?> getClass(String name, Class<?> defaultValue) {
-    String valueString = get(name);
-    if (valueString == null)
-      return defaultValue;
-    try {
-      return getClassByName(valueString);
-    } catch (ClassNotFoundException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
-  @Override
-  public <U> Class<? extends U> getClass(String name,
-      Class<? extends U> defaultValue, Class<U> xface) {
-    try {
-      Class<?> theClass = getClass(name, defaultValue);
-      if (theClass != null && !xface.isAssignableFrom(theClass))
-        throw new RuntimeException(theClass + " not " + xface.getName());
-      else if (theClass != null)
-        return theClass.asSubclass(xface);
-      else
-        return null;
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
-  }
-
-  /*******************************************************************
-   * This class is immutable. Quickly abort any attempts to alter it *
-   *******************************************************************/
-
-  @Override
-  public void clear() {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-
-  @Override
   public void set(String name, String value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setIfUnset(String name, String value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setInt(String name, int value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setLong(String name, long value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setFloat(String name, float value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setBoolean(String name, boolean value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setBooleanIfUnset(String name, boolean value) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setStrings(String name, String... values) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setClass(String name, Class<?> theClass, Class<?> xface) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-  @Override
-  public void setClassLoader(ClassLoader classLoader) {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-
+    if (mutableConf == null) {
+      // not thread safe
+      mutableConf = new Configuration(false); // an empty configuration
+    }
+    mutableConf.set(name,  value);
+  }
+  
+  /***********************************************************************************************
+   * These methods are unsupported, and no code using CompoundConfiguration depend upon them.
+   * Quickly abort upon any attempts to use them.
+   **********************************************************************************************/
   @Override
-  public void readFields(DataInput in) throws IOException {
+  public void clear() {
     throw new UnsupportedOperationException("Immutable Configuration");
   }
 

Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java?rev=1535442&r1=1535441&r2=1535442&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java
(original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java
Thu Oct 24 17:07:55 2013
@@ -50,7 +50,7 @@ public class TestCompoundConfiguration e
   @Test
   public void testBasicFunctionality() throws ClassNotFoundException {
     CompoundConfiguration compoundConf = new CompoundConfiguration()
-        .add(baseConf);
+        .add(baseConf); 
     assertEquals("1", compoundConf.get("A"));
     assertEquals(2, compoundConf.getInt("B", 0));
     assertEquals(3, compoundConf.getInt("C", 0));
@@ -67,6 +67,29 @@ public class TestCompoundConfiguration e
   }
 
   @Test
+  public void testPut() {
+    CompoundConfiguration compoundConf = new CompoundConfiguration()
+      .add(baseConf);
+    assertEquals("1", compoundConf.get("A"));
+    assertEquals(2, compoundConf.getInt("B", 0));
+    assertEquals(3, compoundConf.getInt("C", 0));
+    assertEquals(0, compoundConf.getInt("D", 0));
+
+    compoundConf.set("A", "1337");
+    compoundConf.set("string", "stringvalue");
+    assertEquals(1337, compoundConf.getInt("A", 0));
+    assertEquals("stringvalue", compoundConf.get("string"));
+
+    // we didn't modify the base conf
+    assertEquals("1", baseConf.get("A"));
+    assertNull(baseConf.get("string"));
+
+    // adding to the base shows up in the compound
+    baseConf.set("setInParent", "fromParent");
+    assertEquals("fromParent", compoundConf.get("setInParent"));
+  }
+
+  @Test
   public void testWithConfig() {
     Configuration conf = new Configuration();
     conf.set("B", "2b");
@@ -128,6 +151,52 @@ public class TestCompoundConfiguration e
     }
     // verify that entries from ImmutableConfigMap's are merged in the iterator's view
     assertEquals(baseConfSize + 2, cnt);
+
+    // Verify that adding map after compound configuration is modified overrides properly
+    CompoundConfiguration conf2 = new CompoundConfiguration();
+    conf2.set("X", "modification");
+    conf2.set("D", "not4");
+    assertEquals("modification", conf2.get("X"));
+    assertEquals("not4", conf2.get("D"));
+    conf2.add(map);
+    assertEquals("4", conf2.get("D")); // map overrides
+  }
+
+  @Test
+  public void testWithStringMap() {
+    Map<String, String> map = new HashMap<String, String>();
+    map.put("B", "2b");
+    map.put("C", "33");
+    map.put("D", "4");
+    // unlike config, note that IBW Maps can accept null values
+    map.put("G", null);
+
+    CompoundConfiguration compoundConf = new CompoundConfiguration().addStringMap(map);
+    assertEquals("2b", compoundConf.get("B"));
+    assertEquals(33, compoundConf.getInt("C", 0));
+    assertEquals("4", compoundConf.get("D"));
+    assertEquals(4, compoundConf.getInt("D", 0));
+    assertNull(compoundConf.get("E"));
+    assertEquals(6, compoundConf.getInt("F", 6));
+    assertNull(compoundConf.get("G"));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(4, cnt);
+    
+    // Verify that adding map after compound configuration is modified overrides properly
+    CompoundConfiguration conf2 = new CompoundConfiguration();
+    conf2.set("X", "modification");
+    conf2.set("D", "not4");
+    assertEquals("modification", conf2.get("X"));
+    assertEquals("not4", conf2.get("D"));
+    conf2.addStringMap(map);
+    assertEquals("4", conf2.get("D")); // map overrides
   }
 
   @Test



Mime
View raw message