From 58a8826fb5a0c2d70aabf2a9a99f0b8906124155 Mon Sep 17 00:00:00 2001 From: Thomas Graves Date: Tue, 10 Jul 2012 20:50:17 +0000 Subject: [PATCH] HADOOP-8573. Configuration tries to read from an inputstream resource multiple times (Robert Evans via tgraves) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1359891 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../org/apache/hadoop/conf/Configuration.java | 61 ++++++++++++++----- .../apache/hadoop/conf/TestConfiguration.java | 18 ++++++ 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 7ddb7ef21c9..a847fc389cd 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -791,6 +791,9 @@ Release 0.23.3 - UNRELEASED HADOOP-8129. ViewFileSystemTestSetup setupForViewFileSystem is erring (Ahmed Radwan and Ravi Prakash via bobby) + HADOOP-8573. Configuration tries to read from an inputstream resource + multiple times (Robert Evans via tgraves) + Release 0.23.2 - UNRELEASED INCOMPATIBLE CHANGES 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 f63e0521217..068076a4ef4 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 @@ -44,6 +44,7 @@ import java.util.LinkedList; import java.util.List; import java.util.ListIterator; import java.util.Map; +import java.util.Map.Entry; import java.util.Properties; import java.util.Set; import java.util.StringTokenizer; @@ -611,7 +612,12 @@ public class Configuration implements Iterable>, * The properties of this resource will override properties of previously * added resources, unless they were marked final. * - * @param in InputStream to deserialize the object from. + * WARNING: The contents of the InputStream will be cached, by this method. + * So use this sparingly because it does increase the memory consumption. + * + * @param in InputStream to deserialize the object from. In will be read from + * when a get or set is called next. After it is read the stream will be + * closed. */ public void addResource(InputStream in) { addResourceObject(new Resource(in)); @@ -1837,13 +1843,20 @@ public class Configuration implements Iterable>, } } - for (Resource resource : resources) { - loadResource(properties, resource, quiet); + for (int i = 0; i < resources.size(); i++) { + Resource ret = loadResource(properties, resources.get(i), quiet); + if (ret != null) { + resources.set(i, ret); + } } } - private void loadResource(Properties properties, Resource wrapper, boolean quiet) { + private Resource loadResource(Properties properties, Resource wrapper, boolean quiet) { + String name = UNKNOWN_RESOURCE; try { + Object resource = wrapper.getResource(); + name = wrapper.getName(); + DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); //ignore all comments inside the xml file @@ -1862,9 +1875,7 @@ public class Configuration implements Iterable>, DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); Document doc = null; Element root = null; - - Object resource = wrapper.getResource(); - String name = wrapper.getName(); + boolean returnCachedProperties = false; if (resource instanceof URL) { // an URL resource URL url = (URL)resource; @@ -1901,22 +1912,29 @@ public class Configuration implements Iterable>, } else if (resource instanceof InputStream) { try { doc = builder.parse((InputStream)resource); + returnCachedProperties = true; } finally { ((InputStream)resource).close(); } + } else if (resource instanceof Properties) { + overlay(properties, (Properties)resource); } else if (resource instanceof Element) { root = (Element)resource; } if (doc == null && root == null) { if (quiet) - return; + return null; throw new RuntimeException(resource + " not found"); } if (root == null) { root = doc.getDocumentElement(); } + Properties toAddTo = properties; + if(returnCachedProperties) { + toAddTo = new Properties(); + } if (!"configuration".equals(root.getTagName())) LOG.fatal("bad conf file: top-level element not "); NodeList props = root.getChildNodes(); @@ -1926,7 +1944,7 @@ public class Configuration implements Iterable>, continue; Element prop = (Element)propNode; if ("configuration".equals(prop.getTagName())) { - loadResource(properties, new Resource(prop, name), quiet); + loadResource(toAddTo, new Resource(prop, name), quiet); continue; } if (!"property".equals(prop.getTagName())) @@ -1959,32 +1977,43 @@ public class Configuration implements Iterable>, keyInfo.accessed = false; for (String key:keyInfo.newKeys) { // update new keys with deprecated key's value - loadProperty(properties, name, key, value, finalParameter, + loadProperty(toAddTo, name, key, value, finalParameter, source.toArray(new String[source.size()])); } } else { - loadProperty(properties, name, attr, value, finalParameter, + loadProperty(toAddTo, name, attr, value, finalParameter, source.toArray(new String[source.size()])); } } } - + + if (returnCachedProperties) { + overlay(properties, toAddTo); + return new Resource(toAddTo, name); + } + return null; } catch (IOException e) { - LOG.fatal("error parsing conf file: " + e); + LOG.fatal("error parsing conf " + name, e); throw new RuntimeException(e); } catch (DOMException e) { - LOG.fatal("error parsing conf file: " + e); + LOG.fatal("error parsing conf " + name, e); throw new RuntimeException(e); } catch (SAXException e) { - LOG.fatal("error parsing conf file: " + e); + LOG.fatal("error parsing conf " + name, e); throw new RuntimeException(e); } catch (ParserConfigurationException e) { - LOG.fatal("error parsing conf file: " + e); + LOG.fatal("error parsing conf " + name , e); throw new RuntimeException(e); } } + private void overlay(Properties to, Properties from) { + for (Entry entry: from.entrySet()) { + to.put(entry.getKey(), entry.getValue()); + } + } + private void loadProperty(Properties properties, String name, String attr, String value, boolean finalParameter, String[] source) { if (value != null) { 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 bae8471f8c4..b8e2873c8c1 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 @@ -18,10 +18,12 @@ package org.apache.hadoop.conf; import java.io.BufferedWriter; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.io.InputStream; import java.io.StringWriter; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -77,6 +79,22 @@ public class TestConfiguration extends TestCase { private void addInclude(String filename) throws IOException{ out.write("\n "); } + + public void testInputStreamResource() throws Exception { + StringWriter writer = new StringWriter(); + out = new BufferedWriter(writer); + startConfig(); + declareProperty("prop", "A", "A"); + endConfig(); + + InputStream in1 = new ByteArrayInputStream(writer.toString().getBytes()); + Configuration conf = new Configuration(false); + conf.addResource(in1); + assertEquals("A", conf.get("prop")); + InputStream in2 = new ByteArrayInputStream(writer.toString().getBytes()); + conf.addResource(in2); + assertEquals("A", conf.get("prop")); + } public void testVariableSubstitution() throws IOException { out=new BufferedWriter(new FileWriter(CONFIG));