HADOOP-15708. Reading values from Configuration before adding deprecations make it impossible to read value with deprecated key (zsiegl via rkanter)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/f261c319 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/f261c319 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/f261c319 Branch: refs/heads/HDFS-12943 Commit: f261c319375c5a8c298338752ee77214c22f4e29 Parents: 2bd000c Author: Robert Kanter <rkan...@apache.org> Authored: Wed Oct 10 18:51:37 2018 -0700 Committer: Robert Kanter <rkan...@apache.org> Committed: Wed Oct 10 18:51:37 2018 -0700 ---------------------------------------------------------------------- .../org/apache/hadoop/conf/Configuration.java | 26 +++++- .../conf/TestConfigurationDeprecation.java | 88 ++++++++++++++------ 2 files changed, 88 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/f261c319/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 a78e311..c004cb5 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 @@ -709,6 +709,9 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, * deprecated key, the value of the deprecated key is set as the value for * the provided property name. * + * Also updates properties and overlays with deprecated keys, if the new + * key does not already exist. + * * @param deprecations deprecation context * @param name the property name * @return the first property in the list of properties mapping @@ -730,6 +733,11 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, // Override return value for deprecated keys names = keyInfo.newKeys; } + + // Update properties with deprecated key if already loaded and new + // deprecation has been added + updatePropertiesWithDeprecatedKeys(deprecations, names); + // If there are no overlay values we can return early Properties overlayProperties = getOverlay(); if (overlayProperties.isEmpty()) { @@ -748,6 +756,19 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, } return names; } + + private void updatePropertiesWithDeprecatedKeys( + DeprecationContext deprecations, String[] newNames) { + for (String newName : newNames) { + String deprecatedKey = deprecations.getReverseDeprecatedKeyMap().get(newName); + if (deprecatedKey != null && !getProps().containsKey(newName)) { + String deprecatedValue = getProps().getProperty(deprecatedKey); + if (deprecatedValue != null) { + getProps().setProperty(newName, deprecatedValue); + } + } + } + } private void handleDeprecation() { LOG.debug("Handling deprecation for all properties in config..."); @@ -1187,7 +1208,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>, * the first key which replaces the deprecated key and is not null. * * Values are processed for <a href="#VariableExpansion">variable expansion</a> - * before being returned. + * before being returned. + * + * As a side effect get loads the properties from the sources if called for + * the first time as a lazy init. * * @param name the property name, will be trimmed before get value. * @return the value of the <code>name</code> or its replacing property, http://git-wip-us.apache.org/repos/asf/hadoop/blob/f261c319/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java index 93a235a..4014b60 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java @@ -38,6 +38,7 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import org.apache.hadoop.fs.CommonConfigurationKeys; import org.junit.Assert; import org.apache.hadoop.fs.Path; @@ -52,9 +53,14 @@ import com.google.common.util.concurrent.Uninterruptibles; public class TestConfigurationDeprecation { private Configuration conf; - final static String CONFIG = new File("./test-config-TestConfigurationDeprecation.xml").getAbsolutePath(); - final static String CONFIG2 = new File("./test-config2-TestConfigurationDeprecation.xml").getAbsolutePath(); - final static String CONFIG3 = new File("./test-config3-TestConfigurationDeprecation.xml").getAbsolutePath(); + final static String CONFIG = new File("./test-config" + + "-TestConfigurationDeprecation.xml").getAbsolutePath(); + final static String CONFIG2 = new File("./test-config2" + + "-TestConfigurationDeprecation.xml").getAbsolutePath(); + final static String CONFIG3 = new File("./test-config3" + + "-TestConfigurationDeprecation.xml").getAbsolutePath(); + final static String CONFIG4 = new File("./test-config4" + + "-TestConfigurationDeprecation.xml").getAbsolutePath(); BufferedWriter out; static { @@ -288,14 +294,14 @@ public class TestConfigurationDeprecation { @Test public void testIteratorWithDeprecatedKeys() { Configuration conf = new Configuration(); - Configuration.addDeprecation("dK", new String[]{"nK"}); + Configuration.addDeprecation("dK_iterator", new String[]{"nK_iterator"}); conf.set("k", "v"); - conf.set("dK", "V"); - assertEquals("V", conf.get("dK")); - assertEquals("V", conf.get("nK")); - conf.set("nK", "VV"); - assertEquals("VV", conf.get("dK")); - assertEquals("VV", conf.get("nK")); + conf.set("dK_iterator", "V"); + assertEquals("V", conf.get("dK_iterator")); + assertEquals("V", conf.get("nK_iterator")); + conf.set("nK_iterator", "VV"); + assertEquals("VV", conf.get("dK_iterator")); + assertEquals("VV", conf.get("nK_iterator")); boolean kFound = false; boolean dKFound = false; boolean nKFound = false; @@ -304,11 +310,11 @@ public class TestConfigurationDeprecation { assertEquals("v", entry.getValue()); kFound = true; } - if (entry.getKey().equals("dK")) { + if (entry.getKey().equals("dK_iterator")) { assertEquals("VV", entry.getValue()); dKFound = true; } - if (entry.getKey().equals("nK")) { + if (entry.getKey().equals("nK_iterator")) { assertEquals("VV", entry.getValue()); nKFound = true; } @@ -321,19 +327,19 @@ public class TestConfigurationDeprecation { @Test public void testUnsetWithDeprecatedKeys() { Configuration conf = new Configuration(); - Configuration.addDeprecation("dK", new String[]{"nK"}); - conf.set("nK", "VV"); - assertEquals("VV", conf.get("dK")); - assertEquals("VV", conf.get("nK")); - conf.unset("dK"); - assertNull(conf.get("dK")); - assertNull(conf.get("nK")); - conf.set("nK", "VV"); - assertEquals("VV", conf.get("dK")); - assertEquals("VV", conf.get("nK")); - conf.unset("nK"); - assertNull(conf.get("dK")); - assertNull(conf.get("nK")); + Configuration.addDeprecation("dK_unset", new String[]{"nK_unset"}); + conf.set("nK_unset", "VV"); + assertEquals("VV", conf.get("dK_unset")); + assertEquals("VV", conf.get("nK_unset")); + conf.unset("dK_unset"); + assertNull(conf.get("dK_unset")); + assertNull(conf.get("nK_unset")); + conf.set("nK_unset", "VV"); + assertEquals("VV", conf.get("dK_unset")); + assertEquals("VV", conf.get("nK_unset")); + conf.unset("nK_unset"); + assertNull(conf.get("dK_unset")); + assertNull(conf.get("nK_unset")); } private static String getTestKeyName(int threadIndex, int testIndex) { @@ -426,4 +432,36 @@ public class TestConfigurationDeprecation { assertEquals(null, conf.get("Z")); assertEquals(null, conf.get("X")); } + + @Test + public void testGetPropertyBeforeDeprecetionsAreSet() throws Exception { + // SETUP + final String oldZkAddressKey = "yarn.resourcemanager.zk-address"; + final String newZkAddressKey = CommonConfigurationKeys.ZK_ADDRESS; + final String zkAddressValue = "dummyZkAddress"; + + try{ + out = new BufferedWriter(new FileWriter(CONFIG4)); + startConfig(); + appendProperty(oldZkAddressKey, zkAddressValue); + endConfig(); + + Path fileResource = new Path(CONFIG4); + conf.addResource(fileResource); + } finally { + out.close(); + } + + // ACT + conf.get(oldZkAddressKey); + Configuration.addDeprecations(new Configuration.DeprecationDelta[] { + new Configuration.DeprecationDelta(oldZkAddressKey, newZkAddressKey)}); + + // ASSERT + assertEquals("Property should be accessible through deprecated key", + zkAddressValue, conf.get(oldZkAddressKey)); + assertEquals("Property should be accessible through new key", + zkAddressValue, conf.get(newZkAddressKey)); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
