HADOOP-14701. Configuration can log misleading warnings about an attempt to override final parameter. Contributed by Andrew Sherman.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

@@ -2911,9 +2911,12 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
         if(source != null) {
           updatingResource.put(attr, source);
         }
-      } else if (!value.equals(properties.getProperty(attr))) {
-        LOG.warn(name+":an attempt to override final parameter: "+attr
-            +";  Ignoring.");
+      } else {
+        // This is a final parameter so check for overrides.
+        checkForOverride(this.properties, name, attr, value);
+        if (this.properties != properties) {
+          checkForOverride(properties, name, attr, value);
+        }
       }
     }
     if (finalParameter && attr != null) {
@@ -2921,6 +2924,18 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     }
   }
 
+  /**
+   * Print a warning if a property with a given name already exists with a
+   * different value
+   */
+  private void checkForOverride(Properties properties, String name, String attr, String value) {
+    String propertyValue = properties.getProperty(attr);
+    if (propertyValue != null && !propertyValue.equals(value)) {
+      LOG.warn(name + ":an attempt to override final parameter: " + attr
+          + ";  Ignoring.");
+    }
+  }
+
   /** 
    * Write out the non-default properties in this configuration to the given
    * {@link OutputStream} using UTF-8 encoding.

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java

@@ -36,6 +36,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
@@ -55,6 +56,9 @@ import org.apache.hadoop.test.GenericTestUtils;
 
 import static org.apache.hadoop.util.PlatformName.IBM_JAVA;
 
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Logger;
+import org.apache.log4j.spi.LoggingEvent;
 import org.mockito.Mockito;
 
 public class TestConfiguration extends TestCase {
@@ -161,6 +165,177 @@ public class TestConfiguration extends TestCase {
     assertEquals("A", conf.get("prop"));
   }
 
+  public void testFinalWarnings() throws Exception {
+    // Make a configuration file with a final property
+    StringWriter writer = new StringWriter();
+    out = new BufferedWriter(writer);
+    startConfig();
+    declareProperty("prop", "A", "A", true);
+    endConfig();
+    byte[] bytes = writer.toString().getBytes();
+    InputStream in1 = new ByteArrayInputStream(bytes);
+
+    // Make a second config file with a final property with a different value
+    writer = new StringWriter();
+    out = new BufferedWriter(writer);
+    startConfig();
+    declareProperty("prop", "BB", "BB", true);
+    endConfig();
+    byte[] bytes2 = writer.toString().getBytes();
+    InputStream in2 = new ByteArrayInputStream(bytes2);
+
+    // Attach our own log appender so we can verify output
+    TestAppender appender = new TestAppender();
+    final Logger logger = Logger.getRootLogger();
+    logger.addAppender(appender);
+
+    try {
+      // Add the 2 different resources - this should generate a warning
+      conf.addResource(in1);
+      conf.addResource(in2);
+      assertEquals("should see the first value", "A", conf.get("prop"));
+
+      List<LoggingEvent> events = appender.getLog();
+      assertEquals("overriding a final parameter should cause logging", 1,
+          events.size());
+      LoggingEvent loggingEvent = events.get(0);
+      String renderedMessage = loggingEvent.getRenderedMessage();
+      assertTrue("did not see expected string inside message "+ renderedMessage,
+          renderedMessage.contains("an attempt to override final parameter: "
+              + "prop;  Ignoring."));
+    } finally {
+      // Make sure the appender is removed
+      logger.removeAppender(appender);
+    }
+  }
+
+  public void testNoFinalWarnings() throws Exception {
+    // Make a configuration file with a final property
+    StringWriter writer = new StringWriter();
+    out = new BufferedWriter(writer);
+    startConfig();
+    declareProperty("prop", "A", "A", true);
+    endConfig();
+    byte[] bytes = writer.toString().getBytes();
+    // The 2 input streams both have the same config file
+    InputStream in1 = new ByteArrayInputStream(bytes);
+    InputStream in2 = new ByteArrayInputStream(bytes);
+
+    // Attach our own log appender so we can verify output
+    TestAppender appender = new TestAppender();
+    final Logger logger = Logger.getRootLogger();
+    logger.addAppender(appender);
+
+    try {
+      // Add the resource twice from a stream - should not generate warnings
+      conf.addResource(in1);
+      conf.addResource(in2);
+      assertEquals("A", conf.get("prop"));
+
+      List<LoggingEvent> events = appender.getLog();
+      for (LoggingEvent loggingEvent : events) {
+        System.out.println("Event = " + loggingEvent.getRenderedMessage());
+      }
+      assertTrue("adding same resource twice should not cause logging",
+          events.isEmpty());
+    } finally {
+      // Make sure the appender is removed
+      logger.removeAppender(appender);
+    }
+  }
+
+
+
+  public void testFinalWarningsMultiple() throws Exception {
+    // Make a configuration file with a repeated final property
+    StringWriter writer = new StringWriter();
+    out = new BufferedWriter(writer);
+    startConfig();
+    declareProperty("prop", "A", "A", true);
+    declareProperty("prop", "A", "A", true);
+    endConfig();
+    byte[] bytes = writer.toString().getBytes();
+    InputStream in1 = new ByteArrayInputStream(bytes);
+
+    // Attach our own log appender so we can verify output
+    TestAppender appender = new TestAppender();
+    final Logger logger = Logger.getRootLogger();
+    logger.addAppender(appender);
+
+    try {
+      // Add the resource - this should not produce a warning
+      conf.addResource(in1);
+      assertEquals("should see the value", "A", conf.get("prop"));
+
+      List<LoggingEvent> events = appender.getLog();
+      for (LoggingEvent loggingEvent : events) {
+        System.out.println("Event = " + loggingEvent.getRenderedMessage());
+      }
+      assertTrue("adding same resource twice should not cause logging",
+          events.isEmpty());
+    } finally {
+      // Make sure the appender is removed
+      logger.removeAppender(appender);
+    }
+  }
+
+  public void testFinalWarningsMultipleOverride() throws Exception {
+    // Make a configuration file with 2 final properties with different values
+    StringWriter writer = new StringWriter();
+    out = new BufferedWriter(writer);
+    startConfig();
+    declareProperty("prop", "A", "A", true);
+    declareProperty("prop", "BB", "BB", true);
+    endConfig();
+    byte[] bytes = writer.toString().getBytes();
+    InputStream in1 = new ByteArrayInputStream(bytes);
+
+    // Attach our own log appender so we can verify output
+    TestAppender appender = new TestAppender();
+    final Logger logger = Logger.getRootLogger();
+    logger.addAppender(appender);
+
+    try {
+      // Add the resource - this should produce a warning
+      conf.addResource(in1);
+      assertEquals("should see the value", "A", conf.get("prop"));
+
+      List<LoggingEvent> events = appender.getLog();
+      assertEquals("overriding a final parameter should cause logging", 1,
+          events.size());
+      LoggingEvent loggingEvent = events.get(0);
+      String renderedMessage = loggingEvent.getRenderedMessage();
+      assertTrue("did not see expected string inside message "+ renderedMessage,
+          renderedMessage.contains("an attempt to override final parameter: "
+              + "prop;  Ignoring."));
+    } finally {
+      // Make sure the appender is removed
+      logger.removeAppender(appender);
+    }
+  }
+
+  /**
+   * A simple appender for white box testing.
+   */
+  private static class TestAppender extends AppenderSkeleton {
+    private final List<LoggingEvent> log = new ArrayList<>();
+
+    @Override public boolean requiresLayout() {
+      return false;
+    }
+
+    @Override protected void append(final LoggingEvent loggingEvent) {
+      log.add(loggingEvent);
+    }
+
+    @Override public void close() {
+    }
+
+    public List<LoggingEvent> getLog() {
+      return new ArrayList<>(log);
+    }
+  }
+
   /**
    * Tests use of multi-byte characters in property names and values.  This test
    * round-trips multi-byte string literals through saving and loading of config