You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by wa...@apache.org on 2017/08/01 22:18:28 UTC
hadoop git commit: HADOOP-14701. Configuration can log misleading
warnings about an attempt to override final parameter. Contributed by Andrew
Sherman.
Repository: hadoop
Updated Branches:
refs/heads/trunk 778d4edd9 -> a11c23023
HADOOP-14701. Configuration can log misleading warnings about an attempt to override final parameter. Contributed by Andrew Sherman.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/a11c2302
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/a11c2302
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/a11c2302
Branch: refs/heads/trunk
Commit: a11c230236036317a6c12deeca401e3ae8dce698
Parents: 778d4ed
Author: Andrew Wang <wa...@apache.org>
Authored: Tue Aug 1 15:13:29 2017 -0700
Committer: Andrew Wang <wa...@apache.org>
Committed: Tue Aug 1 15:13:29 2017 -0700
----------------------------------------------------------------------
.../org/apache/hadoop/conf/Configuration.java | 21 ++-
.../apache/hadoop/conf/TestConfiguration.java | 175 +++++++++++++++++++
2 files changed, 193 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/a11c2302/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
index de52fbb..e26d3a8 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
+++ b/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.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/a11c2302/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
index 5ced541..2af61c0 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
+++ b/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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org