HADOOP-785. Add support for 'final' Configuration parameters. Contributed by Arun.

git-svn-id: https://svn.apache.org/repos/asf/lucene/hadoop/trunk@574654 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -22,6 +22,13 @@ Trunk (unreleased changes)
     HADOOP-1838.  The blocksize of files created with an earlier release is
     set to the default block size.  (Dhruba Borthakur via dhruba)
 
+    HADOOP-785.  Add support for 'final' Configuration parameters,
+    removing support for 'mapred-default.xml', and changing
+    'hadoop-site.xml' to not override other files.  Now folks should
+    generally use 'hadoop-site.xml' for all configurations.  Values
+    with a 'final' tag may not be overridden by subsequently loaded
+    configuration files, e.g., by jobs.  (Arun C. Murthy via cutting)
+
   NEW FEATURES
 
     HADOOP-1636.  Allow configuration of the number of jobs kept in

build.xml

@@ -346,7 +346,6 @@
         </section>
       </manifest>
       <fileset file="${conf.dir}/hadoop-default.xml"/>
-      <fileset file="${conf.dir}/mapred-default.xml"/>
       <fileset file="${conf.dir}/commons-logging.properties"/>
       <fileset file="${conf.dir}/log4j.properties"/>
       <fileset file="${conf.dir}/hadoop-metrics.properties"/>

conf/mapred-default.xml.template

@@ -1,8 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
-
-<!-- Put mapred-specific property overrides in this file. -->
-
-<configuration>
-
-</configuration>

src/contrib/ec2/bin/image/hadoop-init

@@ -29,15 +29,6 @@ cat > $HADOOP_HOME/conf/hadoop-site.xml <<EOF
   <value>$MASTER_HOST:50002</value>
 </property>
 
-</configuration>
-EOF
-
-cat > $HADOOP_HOME/conf/mapred-default.xml <<EOF
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
-
-<configuration>
-
 <property>
   <name>mapred.map.tasks</name>
   <value>$(( NO_INSTANCES * 10 ))</value>

src/contrib/test/mapred-default.xml

@@ -1,10 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="nutch-conf.xsl"?>
-
-<!-- Values used when running unit tests.  This is mostly empty, to -->
-<!-- use of the default values, overriding the potentially -->
-<!-- user-editted mapred-default.xml in the conf/ directory.  -->
-
-<configuration>
-
-</configuration>

src/java/org/apache/hadoop/conf/Configuration.java

@@ -46,13 +46,20 @@ import org.apache.hadoop.fs.Path;
  * File, then the local filesystem is examined directly, without referring to
  * the CLASSPATH.
  *
- * <p>Configuration resources are of two types: default and
- * final.  Default values are loaded first and final values are loaded last, and
- * thus override default values.
+ * <p>Configuration parameters may be declared 'final'.  Once a resource
+ * declares a value final, no subsequently-loaded resource may alter that
+ * value.  For example, one might define a final parameter with:
+ * <pre>
+ *  &lt;property&gt;
+ *    &lt;name&gt;dfs.client.buffer.dir&lt;/name&gt;
+ *    &lt;value&gt;/tmp/hadoop/dfs/client&lt;/value&gt;
+ *    &lt;final&gt;true&lt;/final&gt;
+ *  &lt;/property&gt;
+ * </pre>
  *
- * <p>Hadoop's default resource is the String "hadoop-default.xml" and its
- * final resource is the String "hadoop-site.xml".  Other tools built on Hadoop
- * may specify additional resources.
+ * <p>Hadoop by default specifies two resource strings: "hadoop-default.xml"
+ * and "hadoop-site.xml".  Other tools built on Hadoop may specify additional
+ * resources.
  * 
  * <p>The values returned by most <tt>get*</tt> methods are based on String representations. 
  * This String is processed for <b>variable expansion</b>. The available variables are the 
@@ -73,10 +80,27 @@ public class Configuration implements Iterable<Map.Entry<String,String>> {
   private static final Log LOG =
     LogFactory.getLog("org.apache.hadoop.conf.Configuration");
 
-  private boolean   quietmode = true;
+  private boolean quietmode = true;
+  
+  /**
+   * @deprecated Remove in hadoop-0.16.0 via HADOOP-1843
+   */
   private ArrayList<Object> defaultResources = new ArrayList<Object>();
+  /**
+   * @deprecated Remove in hadoop-0.16.0 via HADOOP-1843
+   */
   private ArrayList<Object> finalResources = new ArrayList<Object>();
+  
+  /**
+   * List of configuration resources.
+   */
+  private ArrayList<Object> resources = new ArrayList<Object>();
 
+  /**
+   * List of configuration parameters marked <b>final</b>. 
+   */
+  private Set<String> finalParameters = new HashSet<String>();
+  
   private Properties properties;
   private Properties overlay;
   private ClassLoader classLoader;
@@ -105,47 +129,92 @@ public class Configuration implements Iterable<Map.Entry<String,String>> {
     }
     this.defaultResources = (ArrayList)other.defaultResources.clone();
     this.finalResources = (ArrayList)other.finalResources.clone();
+    this.resources = (ArrayList)other.resources.clone();
     if (other.properties != null)
       this.properties = (Properties)other.properties.clone();
     if (other.overlay!=null)
       this.overlay = (Properties)other.overlay.clone();
+    this.finalParameters = new HashSet<String>(other.finalParameters);
   }
 
-  /** Add a default resource. */
+  /** 
+   * Add a default resource.
+   * @deprecated Use {@link #addResource(String)} instead  
+   */
   public void addDefaultResource(String name) {
     addResource(defaultResources, name);
   }
 
-  /** Add a default resource. */
+  /** 
+   * Add a default resource.
+   * @deprecated Use {@link #addResource(URL)} instead
+   */
   public void addDefaultResource(URL url) {
     addResource(defaultResources, url);
   }
 
-  /** Add a default resource. */
+  /** 
+   * Add a default resource.
+   * @deprecated Use {@link #addResource(Path)} instead  
+   */
   public void addDefaultResource(Path file) {
     addResource(defaultResources, file);
   }
 
-  /** Add a final resource. */
+  /** 
+   * Add a final resource.
+   * @deprecated Use {@link #addResource(String)} instead
+   */
   public void addFinalResource(String name) {
     addResource(finalResources, name);
   }
 
-  /** Add a final resource. */
+  /** 
+   * Add a final resource.
+   * @deprecated Use {@link #addResource(URL)} instead  
+   */
   public void addFinalResource(URL url) {
     addResource(finalResources, url);
   }
 
-  /** Add a final resource. */
+  /** 
+   * Add a final resource.
+   * @deprecated Use {@link #addResource(Path)} instead
+   */
   public void addFinalResource(Path file) {
     addResource(finalResources, file);
   }
 
+  /**
+   * Add a configuration resource.
+   * @param name resource to be added
+   */
+  public void addResource(String name) {
+    addResource(resources, name);
+  }
+
+  /**
+   * Add a configuration resource. 
+   * @param url url of the resource to be added
+   */
+  public void addResource(URL url) {
+    addResource(resources, url);
+  }
+
+  /**
+   * Add a configuration resource. 
+   * @param file file-path of resource to be added
+   */
+  public void addResource(Path file) {
+    addResource(resources, file);
+  }
+
   private synchronized void addResource(ArrayList<Object> resources,
                                         Object resource) {
     
     resources.add(resource);                      // add to resources
     properties = null;                            // trigger reload
+    finalParameters.clear();                      // clear site-limits
   }
   
   /**
@@ -481,11 +550,11 @@ public class Configuration implements Iterable<Map.Entry<String,String>> {
 
   private synchronized Properties getProps() {
     if (properties == null) {
-      Properties newProps = new Properties();
-      loadResources(newProps, defaultResources, false, quietmode);
-      loadResources(newProps, finalResources, true, true);
-      properties = newProps;
-      if (overlay!=null)
+      properties = new Properties();
+      loadResources(properties, defaultResources, quietmode);
+      loadResources(properties, finalResources, quietmode);
+      loadResources(properties, resources, quietmode);
+      if (overlay!= null)
         properties.putAll(overlay);
     }
     return properties;
@@ -517,12 +586,11 @@ public class Configuration implements Iterable<Map.Entry<String,String>> {
     return result.entrySet().iterator();
   }
 
-  private void loadResources(Properties props,
+  private void loadResources(Properties properties,
                              ArrayList resources,
-                             boolean reverse, boolean quiet) {
-    ListIterator i = resources.listIterator(reverse ? resources.size() : 0);
-    while (reverse ? i.hasPrevious() : i.hasNext()) {
-      loadResource(props, reverse ? i.previous() : i.next(), quiet);
+                             boolean quiet) {
+    for (Object resource : resources) {
+      loadResource(properties, resource, quiet);
     }
   }
 
@@ -587,18 +655,31 @@ public class Configuration implements Iterable<Map.Entry<String,String>> {
         NodeList fields = prop.getChildNodes();
         String attr = null;
         String value = null;
+        boolean finalParameter = false;
         for (int j = 0; j < fields.getLength(); j++) {
           Node fieldNode = fields.item(j);
           if (!(fieldNode instanceof Element))
             continue;
           Element field = (Element)fieldNode;
-          if ("name".equals(field.getTagName()))
+          if ("name".equals(field.getTagName()) && field.hasChildNodes())
             attr = ((Text)field.getFirstChild()).getData();
           if ("value".equals(field.getTagName()) && field.hasChildNodes())
             value = ((Text)field.getFirstChild()).getData();
+          if ("final".equals(field.getTagName()) && field.hasChildNodes())
+            finalParameter = "true".equals(((Text)field.getFirstChild()).getData());
+        }
+        
+        // Ignore this parameter if it has already been marked as 'final'
+        if (attr != null && value != null) {
+          if (!finalParameters.contains(attr)) {
+            properties.setProperty(attr, value);
+            if (finalParameter)
+              finalParameters.add(attr);
+          } else {
+            LOG.warn(name+":a attempt to override final parameter: "+attr
+                     +";  Ignoring.");
+          }
         }
-        if (attr != null && value != null)
-          properties.setProperty(attr, value);
       }
         
     } catch (Exception e) {

src/java/org/apache/hadoop/mapred/JobClient.java

@@ -724,7 +724,7 @@ public class JobClient extends Configured implements MRConstants, Tool  {
         if (validate == null) {
           throw new RuntimeException(classpathFile + " not found on CLASSPATH");
         }
-        conf.addFinalResource(classpathFile);
+        conf.addResource(classpathFile);
       }
     }
     return conf;

src/java/org/apache/hadoop/mapred/JobConf.java

@@ -25,7 +25,6 @@ import java.util.StringTokenizer;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.Map;
 
 import java.net.URL;
 import java.net.URLDecoder;
@@ -47,24 +46,17 @@ import org.apache.hadoop.util.ReflectionUtils;
  * {@link OutputFormat} implementations to be used.  It also indicates the set
  * of input files, and where the output files should be written. */
 public class JobConf extends Configuration {
-
-  private void initialize() {
-    addDefaultResource("mapred-default.xml");
-  }
   
   /**
    * Construct a map/reduce job configuration.
    */
-  public JobConf() {
-    initialize();
-  }
+  public JobConf() {}
 
   /** 
    * Construct a map/reduce job configuration.
    * @param exampleClass a class whose containing jar is used as the job's jar.
    */
   public JobConf(Class exampleClass) {
-    initialize();
     setJarByClass(exampleClass);
   }
   
@@ -75,7 +67,6 @@ public class JobConf extends Configuration {
    */
   public JobConf(Configuration conf) {
     super(conf);
-    initialize();
   }
 
 
@@ -86,7 +77,6 @@ public class JobConf extends Configuration {
    */
   public JobConf(Configuration conf, Class exampleClass) {
     this(conf);
-    initialize();
     setJarByClass(exampleClass);
   }
 
@@ -105,8 +95,7 @@ public class JobConf extends Configuration {
    */
   public JobConf(Path config) {
     super();
-    addDefaultResource("mapred-default.xml");
-    addDefaultResource(config);
+    addResource(config);
   }
 
   public String getJar() { return get("mapred.jar"); }

src/java/org/apache/hadoop/mapred/TaskTracker.java

@@ -1774,7 +1774,7 @@ public class TaskTracker
       TaskLog.cleanup(job.getInt("mapred.userlog.retain.hours", 24));
       task.setConf(job);
           
-      defaultConf.addFinalResource(new Path(task.getJobFile()));
+      defaultConf.addResource(new Path(task.getJobFile()));
       
       // Initiate Java VM metrics
       JvmMetrics.init(task.getPhase().toString(), job.getSessionId());

src/java/org/apache/hadoop/mapred/lib/aggregate/ValueAggregatorJob.java

@@ -143,7 +143,7 @@ public class ValueAggregatorJob {
     
     JobConf theJob = new JobConf(conf);
     if (specFile != null) {
-      theJob.addDefaultResource(specFile);
+      theJob.addResource(specFile);
     }
     String userJarFile = theJob.get("user.jar.file");
     if (userJarFile == null) {

src/java/org/apache/hadoop/mapred/pipes/Submitter.java

@@ -341,7 +341,7 @@ public class Submitter {
       CommandLine results = parser.parse(args);
       JobConf conf = new JobConf();
       if (results.hasOption("-conf")) {
-        conf.addFinalResource(new Path((String) results.getValue("-conf")));
+        conf.addResource(new Path((String) results.getValue("-conf")));
       }
       if (results.hasOption("-input")) {
         conf.setInputPath(new Path((String) results.getValue("-input")));

src/java/org/apache/hadoop/util/GenericOptionsParser.java

@@ -177,7 +177,7 @@ public class GenericOptionsParser {
       conf.set("mapred.job.tracker", line.getOptionValue("jt"));
     }
     if (line.hasOption("conf")) {
-      conf.addFinalResource(new Path(line.getOptionValue("conf")));
+      conf.addResource(new Path(line.getOptionValue("conf")));
     }
     if (line.hasOption('D')) {
       String[] property = line.getOptionValues('D');

src/java/overview.html

@@ -198,7 +198,7 @@ MapReduce data is written.  It also may be a list of directories.</li>
 href="../hadoop-default.html#mapred.map.tasks">mapred.map.tasks</a></tt>
 and <tt><a
 href="../hadoop-default.html#mapred.reduce.tasks">mapred.reduce.tasks</a></tt>
-in <tt>conf/mapred-default.xml</tt>.  As a rule of thumb, use 10x the
+in <tt>conf/hadoop-site.xml</tt>.  As a rule of thumb, use 10x the
 number of slave processors for <tt>mapred.map.tasks</tt>, and 2x the
 number of slave processors for <tt>mapred.reduce.tasks</tt>.</li>
 

src/test/hadoop-site.xml

@@ -12,6 +12,7 @@
   <name>hadoop.tmp.dir</name>
   <value>build/test</value>
   <description>A base for other temporary directories.</description>
+  <final>true</final>
 </property>
 
 <property>

src/test/mapred-default.xml

@@ -1,10 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="nutch-conf.xsl"?>
-
-<!-- Values used when running unit tests.  This is mostly empty, to -->
-<!-- use of the default values, overriding the potentially -->
-<!-- user-editted mapred-default.xml in the conf/ directory.  -->
-
-<configuration>
-
-</configuration>

src/test/org/apache/hadoop/conf/TestConfiguration.java

@@ -69,7 +69,7 @@ public class TestConfiguration extends TestCase {
     declareProperty("my.failsexpand", "a${my.undefvar}b", "a${my.undefvar}b");
     endConfig();
     Path fileResource = new Path(CONFIG);
-    conf.addDefaultResource(fileResource);
+    conf.addResource(fileResource);
 
     for (Prop p : props) {
       System.out.println("p=" + p.name);
@@ -106,7 +106,13 @@ public class TestConfiguration extends TestCase {
 
   void declareProperty(String name, String val, String expectEval)
     throws IOException {
-    appendProperty(name, val);
+    declareProperty(name, val, expectEval, false);
+  }
+
+  void declareProperty(String name, String val, String expectEval,
+                       boolean isFinal)
+    throws IOException {
+    appendProperty(name, val, isFinal);
     Prop p = new Prop();
     p.name = name;
     p.val = val;
@@ -115,6 +121,11 @@ public class TestConfiguration extends TestCase {
   }
 
   void appendProperty(String name, String val) throws IOException {
+    appendProperty(name, val, false);
+  }
+ 
+  void appendProperty(String name, String val, boolean isFinal)
+    throws IOException {
     out.write("<property>");
     out.write("<name>");
     out.write(name);
@@ -122,6 +133,9 @@ public class TestConfiguration extends TestCase {
     out.write("<value>");
     out.write(val);
     out.write("</value>");
+    if (isFinal) {
+      out.write("<final>true</final>");
+    }
     out.write("</property>\n");
   }
   
@@ -131,30 +145,31 @@ public class TestConfiguration extends TestCase {
     appendProperty("a","b");
     appendProperty("b","c");
     appendProperty("d","e");
+    appendProperty("e","f", true);
     endConfig();
 
     out=new BufferedWriter(new FileWriter(CONFIG2));
     startConfig();
     appendProperty("a","b");
     appendProperty("b","d");
+    appendProperty("e","e");
     endConfig();
-
-    
     
     Path fileResource = new Path(CONFIG);
-    conf.addDefaultResource(fileResource);
+    conf.addResource(fileResource);
     
     //set dynamically something
     conf.set("c","d");
     conf.set("a","d");
     
     Configuration clone=new Configuration(conf);
-    clone.addFinalResource(new Path(CONFIG2));
+    clone.addResource(new Path(CONFIG2));
     
     assertEquals(clone.get("a"), "d"); 
     assertEquals(clone.get("b"), "d"); 
     assertEquals(clone.get("c"), "d"); 
     assertEquals(clone.get("d"), "e"); 
+    assertEquals(clone.get("e"), "f"); 
     
   }