Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B027FD774 for ; Mon, 30 Jul 2012 14:21:26 +0000 (UTC) Received: (qmail 71877 invoked by uid 500); 30 Jul 2012 14:21:26 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 71474 invoked by uid 500); 30 Jul 2012 14:21:25 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 71466 invoked by uid 99); 30 Jul 2012 14:21:25 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 30 Jul 2012 14:21:25 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 30 Jul 2012 14:21:21 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 3D6AD2388906; Mon, 30 Jul 2012 14:20:36 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1367102 - in /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common: ./ src/main/java/org/apache/hadoop/conf/ src/test/java/org/apache/hadoop/conf/ Date: Mon, 30 Jul 2012 14:20:35 -0000 To: common-commits@hadoop.apache.org From: bobby@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120730142036.3D6AD2388906@eris.apache.org> Author: bobby Date: Mon Jul 30 14:20:35 2012 New Revision: 1367102 URL: http://svn.apache.org/viewvc?rev=1367102&view=rev Log: svn merge -c 1301551,1300642,1332821,1303884 FIXES:MAPREDUCE-4010,HADOOP-8167,HADOOP-8172,HADOOP-8197 Patches to fix OOZIE-761 Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1367102&r1=1367101&r2=1367102&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt Mon Jul 30 14:20:35 2012 @@ -123,6 +123,13 @@ Release 0.23.3 - UNRELEASED HADOOP-8613. AbstractDelegationTokenIdentifier#getUser() should set token auth type. (daryn) + HADOOP-8167. Configuration deprecation logic breaks backwards compatibility (tucu) + + HADOOP-8172. Configuration no longer sets all keys in a deprecated key + list. (Anupam Seth via bobby) + + HADOOP-8197. Configuration logs WARNs on every use of a deprecated key (tucu) + Release 0.23.2 - UNRELEASED NEW FEATURES Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java?rev=1367102&r1=1367101&r2=1367102&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java Mon Jul 30 14:20:35 2012 @@ -293,10 +293,18 @@ public class Configuration implements It * This is to be used only by the developers in order to add deprecation of * keys, and attempts to call this method after loading resources once, * would lead to UnsupportedOperationException + * + * If a key is deprecated in favor of multiple keys, they are all treated as + * aliases of each other, and setting any one of them resets all the others + * to the new value. + * * @param key * @param newKeys * @param customMessage + * @deprecated use {@link addDeprecation(String key, String newKey, + String customMessage)} instead */ + @Deprecated public synchronized static void addDeprecation(String key, String[] newKeys, String customMessage) { if (key == null || key.length() == 0 || @@ -312,6 +320,22 @@ public class Configuration implements It } } } + + /** + * Adds the deprecated key to the deprecation map. + * It does not override any existing entries in the deprecation map. + * This is to be used only by the developers in order to add deprecation of + * keys, and attempts to call this method after loading resources once, + * would lead to UnsupportedOperationException + * + * @param key + * @param newKey + * @param customMessage + */ + public synchronized static void addDeprecation(String key, String newKey, + String customMessage) { + addDeprecation(key, new String[] {newKey}, customMessage); + } /** * Adds the deprecated key to the deprecation map when no custom message @@ -321,24 +345,74 @@ public class Configuration implements It * keys, and attempts to call this method after loading resources once, * would lead to UnsupportedOperationException * + * If a key is deprecated in favor of multiple keys, they are all treated as + * aliases of each other, and setting any one of them resets all the others + * to the new value. + * * @param key Key that is to be deprecated * @param newKeys list of keys that take up the values of deprecated key + * @deprecated use {@link addDeprecation(String key, String newKey)} instead */ + @Deprecated public synchronized static void addDeprecation(String key, String[] newKeys) { addDeprecation(key, newKeys, null); } /** + * Adds the deprecated key to the deprecation map when no custom message + * is provided. + * It does not override any existing entries in the deprecation map. + * This is to be used only by the developers in order to add deprecation of + * keys, and attempts to call this method after loading resources once, + * would lead to UnsupportedOperationException + * + * @param key Key that is to be deprecated + * @param newKey key that takes up the value of deprecated key + */ + public synchronized static void addDeprecation(String key, String newKey) { + addDeprecation(key, new String[] {newKey}, null); + } + + /** * checks whether the given key is deprecated. * * @param key the parameter which is to be checked for deprecation * @return true if the key is deprecated and * false otherwise. */ - private static boolean isDeprecated(String key) { + public static boolean isDeprecated(String key) { return deprecatedKeyMap.containsKey(key); } - + + /** + * Returns the alternate name for a key if the property name is deprecated + * or if deprecates a property name. + * + * @param name property name. + * @return alternate name. + */ + private String[] getAlternateNames(String name) { + String oldName, altNames[] = null; + DeprecatedKeyInfo keyInfo = deprecatedKeyMap.get(name); + if (keyInfo == null) { + altNames = (reverseDeprecatedKeyMap.get(name) != null ) ? + new String [] {reverseDeprecatedKeyMap.get(name)} : null; + if(altNames != null && altNames.length > 0) { + //To help look for other new configs for this deprecated config + keyInfo = deprecatedKeyMap.get(altNames[0]); + } + } + if(keyInfo != null && keyInfo.newKeys.length > 0) { + List list = new ArrayList(); + if(altNames != null) { + list.addAll(Arrays.asList(altNames)); + } + list.addAll(Arrays.asList(keyInfo.newKeys)); + altNames = list.toArray(new String[list.size()]); + } + return altNames; + } + /** * Checks for the presence of the property name in the * deprecation map. Returns the first of the list of new keys if present @@ -351,31 +425,29 @@ public class Configuration implements It * @return the first property in the list of properties mapping * the name or the name itself. */ - private String handleDeprecation(String name) { - if (isDeprecated(name)) { + private String[] handleDeprecation(String name) { + ArrayList names = new ArrayList(); + if (isDeprecated(name)) { DeprecatedKeyInfo keyInfo = deprecatedKeyMap.get(name); - if (!keyInfo.accessed) { - LOG.warn(keyInfo.getWarningMessage(name)); - } + warnOnceIfDeprecated(name); for (String newKey : keyInfo.newKeys) { if(newKey != null) { - name = newKey; - break; + names.add(newKey); } } } - String deprecatedKey = reverseDeprecatedKeyMap.get(name); - if (deprecatedKey != null && !getOverlay().containsKey(name) && - getOverlay().containsKey(deprecatedKey)) { - getProps().setProperty(name, getOverlay().getProperty(deprecatedKey)); - getOverlay().setProperty(name, getOverlay().getProperty(deprecatedKey)); - - DeprecatedKeyInfo keyInfo = deprecatedKeyMap.get(deprecatedKey); - if (!keyInfo.accessed) { - LOG.warn(keyInfo.getWarningMessage(deprecatedKey)); - } + if(names.size() == 0) { + names.add(name); + } + for(String n : names) { + String deprecatedKey = reverseDeprecatedKeyMap.get(n); + if (deprecatedKey != null && !getOverlay().containsKey(n) && + getOverlay().containsKey(deprecatedKey)) { + getProps().setProperty(n, getOverlay().getProperty(deprecatedKey)); + getOverlay().setProperty(n, getOverlay().getProperty(deprecatedKey)); + } } - return name; + return names.toArray(new String[names.size()]); } private void handleDeprecation() { @@ -626,8 +698,12 @@ public class Configuration implements It * or null if no such property exists. */ public String get(String name) { - name = handleDeprecation(name); - return substituteVars(getProps().getProperty(name)); + String[] names = handleDeprecation(name); + String result = null; + for(String n : names) { + result = substituteVars(getProps().getProperty(n)); + } + return result; } /** @@ -664,14 +740,18 @@ public class Configuration implements It * its replacing property and null if no such property exists. */ public String getRaw(String name) { - name = handleDeprecation(name); - return getProps().getProperty(name); + String[] names = handleDeprecation(name); + String result = null; + for(String n : names) { + result = getProps().getProperty(n); + } + return result; } /** * Set the value of the name property. If - * name is deprecated, it sets the value to the keys - * that replace the deprecated key. + * name is deprecated or there is a deprecated name associated to it, + * it sets the value to both names. * * @param name property name. * @param value property value. @@ -694,37 +774,47 @@ public class Configuration implements It if (deprecatedKeyMap.isEmpty()) { getProps(); } - if (!isDeprecated(name)) { - getOverlay().setProperty(name, value); - getProps().setProperty(name, value); - if(source == null) { - updatingResource.put(name, new String[] {"programatically"}); - } else { - updatingResource.put(name, new String[] {source}); - } + getOverlay().setProperty(name, value); + getProps().setProperty(name, value); + if(source == null) { + updatingResource.put(name, new String[] {"programatically"}); + } else { + updatingResource.put(name, new String[] {source}); } - else { - DeprecatedKeyInfo keyInfo = deprecatedKeyMap.get(name); + String[] altNames = getAlternateNames(name); + if (altNames != null && altNames.length > 0) { String altSource = "because " + name + " is deprecated"; - LOG.warn(keyInfo.getWarningMessage(name)); - for (String newKey : keyInfo.newKeys) { - if(!newKey.equals(name)) { - getOverlay().setProperty(newKey, value); - getProps().setProperty(newKey, value); - updatingResource.put(newKey, new String[] {altSource}); + for(String altName : altNames) { + if(!altName.equals(name)) { + getOverlay().setProperty(altName, value); + getProps().setProperty(altName, value); + updatingResource.put(altName, new String[] {altSource}); } } } + warnOnceIfDeprecated(name); } - + + private void warnOnceIfDeprecated(String name) { + DeprecatedKeyInfo keyInfo = deprecatedKeyMap.get(name); + if (keyInfo != null && !keyInfo.accessed) { + LOG.warn(keyInfo.getWarningMessage(name)); + } + } + /** * Unset a previously set property. */ public synchronized void unset(String name) { - name = handleDeprecation(name); - + String[] altNames = getAlternateNames(name); getOverlay().remove(name); getProps().remove(name); + if (altNames !=null && altNames.length > 0) { + for(String altName : altNames) { + getOverlay().remove(altName); + getProps().remove(altName); + } + } } /** @@ -758,8 +848,12 @@ public class Configuration implements It * doesn't exist. */ public String get(String name, String defaultValue) { - name = handleDeprecation(name); - return substituteVars(getProps().getProperty(name, defaultValue)); + String[] names = handleDeprecation(name); + String result = null; + for(String n : names) { + result = substituteVars(getProps().getProperty(n, defaultValue)); + } + return result; } /** Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java?rev=1367102&r1=1367101&r2=1367102&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java Mon Jul 30 14:20:35 2012 @@ -20,6 +20,7 @@ package org.apache.hadoop.conf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -27,6 +28,7 @@ import java.io.BufferedWriter; import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.util.Map; import org.apache.hadoop.fs.Path; import org.junit.After; @@ -162,7 +164,7 @@ public class TestConfigurationDeprecatio conf.set("Y", "y"); conf.set("Z", "z"); // get old key - assertEquals("y", conf.get("X")); + assertEquals("z", conf.get("X")); } /** @@ -270,4 +272,56 @@ public class TestConfigurationDeprecatio new String[]{ "tests.fake-default.new-key" }); assertEquals("hello", conf.get("tests.fake-default.new-key")); } + + @Test + public void testIteratorWithDeprecatedKeys() { + Configuration conf = new Configuration(); + Configuration.addDeprecation("dK", new String[]{"nK"}); + 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")); + boolean kFound = false; + boolean dKFound = false; + boolean nKFound = false; + for (Map.Entry entry : conf) { + if (entry.getKey().equals("k")) { + assertEquals("v", entry.getValue()); + kFound = true; + } + if (entry.getKey().equals("dK")) { + assertEquals("VV", entry.getValue()); + dKFound = true; + } + if (entry.getKey().equals("nK")) { + assertEquals("VV", entry.getValue()); + nKFound = true; + } + } + assertTrue("regular Key not found", kFound); + assertTrue("deprecated Key not found", dKFound); + assertTrue("new Key not found", nKFound); + } + + @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")); + } + } Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java?rev=1367102&r1=1367101&r2=1367102&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java Mon Jul 30 14:20:35 2012 @@ -18,10 +18,15 @@ package org.apache.hadoop.conf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + import java.io.ByteArrayOutputStream; +import java.util.Map; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.junit.Test; import junit.framework.TestCase; @@ -31,6 +36,7 @@ public class TestDeprecatedKeys extends public void testDeprecatedKeys() throws Exception { Configuration conf = new Configuration(); conf.set("topology.script.file.name", "xyz"); + conf.set("topology.script.file.name", "xyz"); String scriptFile = conf.get(CommonConfigurationKeys.NET_TOPOLOGY_SCRIPT_FILE_NAME_KEY); assertTrue(scriptFile.equals("xyz")) ; } @@ -52,4 +58,49 @@ public class TestDeprecatedKeys extends assertTrue(fileContents.contains("old.config.yet.to.be.deprecated")); assertTrue(fileContents.contains("new.conf.to.replace.deprecated.conf")); } + + @Test + public void testIteratorWithDeprecatedKeysMappedToMultipleNewKeys() { + Configuration conf = new Configuration(); + Configuration.addDeprecation("dK", new String[]{"nK1", "nK2"}); + conf.set("k", "v"); + conf.set("dK", "V"); + assertEquals("V", conf.get("dK")); + assertEquals("V", conf.get("nK1")); + assertEquals("V", conf.get("nK2")); + conf.set("nK1", "VV"); + assertEquals("VV", conf.get("dK")); + assertEquals("VV", conf.get("nK1")); + assertEquals("VV", conf.get("nK2")); + conf.set("nK2", "VVV"); + assertEquals("VVV", conf.get("dK")); + assertEquals("VVV", conf.get("nK2")); + assertEquals("VVV", conf.get("nK1")); + boolean kFound = false; + boolean dKFound = false; + boolean nK1Found = false; + boolean nK2Found = false; + for (Map.Entry entry : conf) { + if (entry.getKey().equals("k")) { + assertEquals("v", entry.getValue()); + kFound = true; + } + if (entry.getKey().equals("dK")) { + assertEquals("VVV", entry.getValue()); + dKFound = true; + } + if (entry.getKey().equals("nK1")) { + assertEquals("VVV", entry.getValue()); + nK1Found = true; + } + if (entry.getKey().equals("nK2")) { + assertEquals("VVV", entry.getValue()); + nK2Found = true; + } + } + assertTrue("regular Key not found", kFound); + assertTrue("deprecated Key not found", dKFound); + assertTrue("new Key 1 not found", nK1Found); + assertTrue("new Key 2 not found", nK2Found); + } }