Hadoop Common
  1. Hadoop Common
  2. HADOOP-8573

Configuration tries to read from an inputstream resource multiple times.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.2, 0.23.3, 2.0.0-alpha, 3.0.0
    • Fix Version/s: 0.23.3, 2.0.2-alpha
    • Component/s: conf
    • Labels:
      None

      Description

      If someone calls Configuration.addResource(InputStream) and then reloadConfiguration is called for any reason, Configruation will try to reread the contents of the InputStream, after it has already closed it.

      This never showed up in 1.0 because the framework itself does not call addResource with an InputStream, and typically by the time user code starts running that might call this, all of the default and site resources have already been loaded.

      In 0.23 mapreduce is now a client library, and mapred-site.xml and mapred-default.xml are loaded much later in the process.

      1. HADOOP-8573.txt
        7 kB
        Robert Joseph Evans
      2. HADOOP-8573.txt
        8 kB
        Robert Joseph Evans
      3. HADOOP-8573.txt
        8 kB
        Robert Joseph Evans

        Activity

        Robert Joseph Evans created issue -
        Hide
        Robert Joseph Evans added a comment -

        The only real way to fix this is to cache the contents of the InputStream, or a parsed version of it. The question is how do we reduce the potential memory impact of this? We could change configuration so that it is a list of properties instead of a single combined properties object, but this is a very large change to fix this issue. Because of this I am inclined to not really worry about the memory impact right now. This feature is not that commonly used, and most configuration objects tend to be shared a lot so I don't see the impact being that large.

        Show
        Robert Joseph Evans added a comment - The only real way to fix this is to cache the contents of the InputStream, or a parsed version of it. The question is how do we reduce the potential memory impact of this? We could change configuration so that it is a list of properties instead of a single combined properties object, but this is a very large change to fix this issue. Because of this I am inclined to not really worry about the memory impact right now. This feature is not that commonly used, and most configuration objects tend to be shared a lot so I don't see the impact being that large.
        Hide
        Robert Joseph Evans added a comment -

        This patch just does basic caching on the parsed properties for an InputStream. It replaces the InputStream in the resources array with the properties themselves.

        Show
        Robert Joseph Evans added a comment - This patch just does basic caching on the parsed properties for an InputStream. It replaces the InputStream in the resources array with the properties themselves.
        Robert Joseph Evans made changes -
        Field Original Value New Value
        Attachment HADOOP-8573.txt [ 12535381 ]
        Hide
        Robert Joseph Evans added a comment -

        I forgot to add in that this patch feels like a hack to me, with loadResource returning the properties to be added in, but I could not think of a cleaner way to do it without completely refactoring a lot of that code. If someone else has a better way to do this I would be very happy to switch.

        Show
        Robert Joseph Evans added a comment - I forgot to add in that this patch feels like a hack to me, with loadResource returning the properties to be added in, but I could not think of a cleaner way to do it without completely refactoring a lot of that code. If someone else has a better way to do this I would be very happy to switch.
        Robert Joseph Evans made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12535381/HADOOP-8573.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.ha.TestZKFailoverController
        org.apache.hadoop.metrics2.impl.TestMetricsSystemImpl
        org.apache.hadoop.io.file.tfile.TestTFileByteArrays
        org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1178//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1178//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12535381/HADOOP-8573.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverController org.apache.hadoop.metrics2.impl.TestMetricsSystemImpl org.apache.hadoop.io.file.tfile.TestTFileByteArrays org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1178//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1178//console This message is automatically generated.
        Hide
        Thomas Graves added a comment -

        Mostly looks good, a couple minor things.

        nits:

        • add a space in Configuration.java line 1772 and 1905 after if before ( and 1926 between for and (

        suggestions:

        • could add a comment describing new behavior when using InputStreams - perhaps a warning about memory usage
        • we might change org.apache.hadoop.mapreduce.v2.app.job.impl.JobImpl.loadConfFile to use addResource(Path file) instead of the inputStream. Only used in one place right now but might be better to change it in case it gets used more.
        Show
        Thomas Graves added a comment - Mostly looks good, a couple minor things. nits: add a space in Configuration.java line 1772 and 1905 after if before ( and 1926 between for and ( suggestions: could add a comment describing new behavior when using InputStreams - perhaps a warning about memory usage we might change org.apache.hadoop.mapreduce.v2.app.job.impl.JobImpl.loadConfFile to use addResource(Path file) instead of the inputStream. Only used in one place right now but might be better to change it in case it gets used more.
        Hide
        Robert Joseph Evans added a comment -

        I have addressed your comments with this latest patch. I did not change the behavior of reading in the configuration form HDFS, because Configuration.addResource(Path) only works for the local file system file://.

        Show
        Robert Joseph Evans added a comment - I have addressed your comments with this latest patch. I did not change the behavior of reading in the configuration form HDFS, because Configuration.addResource(Path) only works for the local file system file:// .
        Robert Joseph Evans made changes -
        Attachment HADOOP-8573.txt [ 12535868 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12535868/HADOOP-8573.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.ha.TestZKFailoverController
        org.apache.hadoop.io.file.tfile.TestTFileByteArrays
        org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1181//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1181//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12535868/HADOOP-8573.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverController org.apache.hadoop.io.file.tfile.TestTFileByteArrays org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1181//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1181//console This message is automatically generated.
        Hide
        Robert Joseph Evans added a comment -

        Upmerged the changes to deal with another patch that was just merged in. Oh and the test failures are unrelated.

        Show
        Robert Joseph Evans added a comment - Upmerged the changes to deal with another patch that was just merged in. Oh and the test failures are unrelated.
        Robert Joseph Evans made changes -
        Attachment HADOOP-8573.txt [ 12535885 ]
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12535885/HADOOP-8573.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1182//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1182//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12535885/HADOOP-8573.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1182//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1182//console This message is automatically generated.
        Hide
        Thomas Graves added a comment -

        +1. Thanks Bobby!

        Show
        Thomas Graves added a comment - +1. Thanks Bobby!
        Thomas Graves made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Fix Version/s 0.23.3 [ 12320059 ]
        Fix Version/s 2.0.1-alpha [ 12321441 ]
        Fix Version/s 3.0.0 [ 12320357 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1100 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1100/)
        HADOOP-8573. Configuration tries to read from an inputstream resource multiple times (Robert Evans via tgraves) (Revision 1359891)

        Result = FAILURE
        tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359891
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1100 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1100/ ) HADOOP-8573 . Configuration tries to read from an inputstream resource multiple times (Robert Evans via tgraves) (Revision 1359891) Result = FAILURE tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359891 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #310 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/310/)
        merge -r 1359891:1359892 from branch-2. FIXES: HADOOP-8573 (Revision 1359896)

        Result = UNSTABLE
        tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359896
        Files :

        • /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/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #310 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/310/ ) merge -r 1359891:1359892 from branch-2. FIXES: HADOOP-8573 (Revision 1359896) Result = UNSTABLE tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359896 Files : /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/TestConfiguration.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1133 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1133/)
        HADOOP-8573. Configuration tries to read from an inputstream resource multiple times (Robert Evans via tgraves) (Revision 1359891)

        Result = SUCCESS
        tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359891
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1133 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1133/ ) HADOOP-8573 . Configuration tries to read from an inputstream resource multiple times (Robert Evans via tgraves) (Revision 1359891) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1359891 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Arun C Murthy made changes -
        Fix Version/s 2.0.2-alpha [ 12322473 ]
        Fix Version/s 3.0.0 [ 12320357 ]
        Fix Version/s 2.1.0-alpha [ 12321441 ]
        Arun C Murthy made changes -
        Affects Version/s 2.0.0-alpha [ 12320352 ]
        Affects Version/s 2.1.0-alpha [ 12321441 ]
        Arun C Murthy made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Robert Joseph Evans
            Reporter:
            Robert Joseph Evans
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development