Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12663

Remove Hard-Coded Values From FileSystem.java

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.9.0, 3.0.0-alpha1
    • Component/s: fs
    • Labels:
      None
    • Flags:
      Patch

      Description

      Within FileSystem.java, there is one instance where the global variables "CommonConfigurationKeysPublic.IO_FILE_BUFFER_SIZE_KEY" and "CommonConfigurationKeysPublic.IO_FILE_BUFFER_SIZE_DEFAULT" were being used, but in all other instances, their literal values were being used.

      Please find attached a patch to remove use of literal values and instead replace them with references to the global variables.

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #9067 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9067/)
        HADOOP-12663. Remove Hard-Coded Values From FileSystem.java. (BELUGA (stevel: rev 3fcdbe076fc2bffbe262ba2b2dbd99dc67df4839)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9067 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9067/ ) HADOOP-12663 . Remove Hard-Coded Values From FileSystem.java. (BELUGA (stevel: rev 3fcdbe076fc2bffbe262ba2b2dbd99dc67df4839) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        stevel@apache.org Steve Loughran added a comment -

        +1
        Committed to branch-2+. Thanks!

        Show
        stevel@apache.org Steve Loughran added a comment - +1 Committed to branch-2+. Thanks!
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 mvninstall 8m 24s trunk passed
        +1 compile 9m 10s trunk passed with JDK v1.8.0_66
        +1 compile 9m 37s trunk passed with JDK v1.7.0_91
        +1 checkstyle 0m 17s trunk passed
        +1 mvnsite 1m 7s trunk passed
        +1 mvneclipse 0m 14s trunk passed
        +1 findbugs 1m 57s trunk passed
        +1 javadoc 0m 57s trunk passed with JDK v1.8.0_66
        +1 javadoc 1m 9s trunk passed with JDK v1.7.0_91
        +1 mvninstall 1m 39s the patch passed
        +1 compile 9m 5s the patch passed with JDK v1.8.0_66
        -1 javac 14m 49s root-jdk1.8.0_66 with JDK v1.8.0_66 generated 1 new issues (was 730, now 730).
        +1 javac 9m 5s the patch passed
        +1 compile 9m 37s the patch passed with JDK v1.7.0_91
        +1 javac 9m 37s the patch passed
        -1 checkstyle 0m 17s Patch generated 1 new checkstyle issues in hadoop-common-project/hadoop-common (total was 140, now 139).
        +1 mvnsite 1m 7s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 findbugs 2m 8s the patch passed
        +1 javadoc 0m 59s the patch passed with JDK v1.8.0_66
        +1 javadoc 1m 9s the patch passed with JDK v1.7.0_91
        -1 unit 8m 0s hadoop-common in the patch failed with JDK v1.8.0_66.
        -1 unit 7m 58s hadoop-common in the patch failed with JDK v1.7.0_91.
        +1 asflicense 0m 25s Patch does not generate ASF License warnings.
        76m 40s



        Reason Tests
        JDK v1.8.0_66 Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics
          hadoop.fs.shell.TestCopyPreserveFlag
        JDK v1.7.0_91 Failed junit tests hadoop.ipc.TestIPC



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12779095/FileSystem.HADOOP-12663.0002.patch
        JIRA Issue HADOOP-12663
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux f5432e4c3981 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 5c0ff69
        findbugs v3.0.0
        javac root-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_66.txt
        checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
        unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt
        unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt
        unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt
        JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/testReport/
        modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
        Max memory used 75MB
        Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 8m 24s trunk passed +1 compile 9m 10s trunk passed with JDK v1.8.0_66 +1 compile 9m 37s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 17s trunk passed +1 mvnsite 1m 7s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 57s trunk passed +1 javadoc 0m 57s trunk passed with JDK v1.8.0_66 +1 javadoc 1m 9s trunk passed with JDK v1.7.0_91 +1 mvninstall 1m 39s the patch passed +1 compile 9m 5s the patch passed with JDK v1.8.0_66 -1 javac 14m 49s root-jdk1.8.0_66 with JDK v1.8.0_66 generated 1 new issues (was 730, now 730). +1 javac 9m 5s the patch passed +1 compile 9m 37s the patch passed with JDK v1.7.0_91 +1 javac 9m 37s the patch passed -1 checkstyle 0m 17s Patch generated 1 new checkstyle issues in hadoop-common-project/hadoop-common (total was 140, now 139). +1 mvnsite 1m 7s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 8s the patch passed +1 javadoc 0m 59s the patch passed with JDK v1.8.0_66 +1 javadoc 1m 9s the patch passed with JDK v1.7.0_91 -1 unit 8m 0s hadoop-common in the patch failed with JDK v1.8.0_66. -1 unit 7m 58s hadoop-common in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 76m 40s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics   hadoop.fs.shell.TestCopyPreserveFlag JDK v1.7.0_91 Failed junit tests hadoop.ipc.TestIPC Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12779095/FileSystem.HADOOP-12663.0002.patch JIRA Issue HADOOP-12663 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f5432e4c3981 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5c0ff69 findbugs v3.0.0 javac root-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_66.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8291/console This message was automatically generated.
        Hide
        belugabehr BELUGA BEHR added a comment -

        Thank you for your interest. I have attached a new patch. It still has the wildcard. Going forward, I will remove the wildcard.

        I will perform the work for the rest of the instances in another, more exhaustive, ticket. Thanks.

        Show
        belugabehr BELUGA BEHR added a comment - Thank you for your interest. I have attached a new patch. It still has the wildcard. Going forward, I will remove the wildcard. I will perform the work for the rest of the instances in another, more exhaustive, ticket. Thanks.
        Hide
        arpitagarwal Arpit Agarwal added a comment -

        Hi BELUGA BEHR, thanks for the updated patch. We can replace the wildcard import static with individual imports. The changes look fine otherwise.

        Please consider submitting a single patch so we can get a Jenkins run. If you want to fix usages in other files that would be great too.

        Show
        arpitagarwal Arpit Agarwal added a comment - Hi BELUGA BEHR , thanks for the updated patch. We can replace the wildcard import static with individual imports. The changes look fine otherwise. Please consider submitting a single patch so we can get a Jenkins run. If you want to fix usages in other files that would be great too.
        Hide
        belugabehr BELUGA BEHR added a comment -

        I have attached another patch. I apologize, you'll have to apply the patches in order to get the final affect.

        How about this? Once I understand exactly what the formatting should look like (i.e., this patch is accepted), I'll create a new ticket and work on the rest.

        Thanks!

        Show
        belugabehr BELUGA BEHR added a comment - I have attached another patch. I apologize, you'll have to apply the patches in order to get the final affect. How about this? Once I understand exactly what the formatting should look like (i.e., this patch is accepted), I'll create a new ticket and work on the rest. Thanks!
        Hide
        stevel@apache.org Steve Loughran added a comment -

        consider static importing the keys and then refer to them without the prefix

        Show
        stevel@apache.org Steve Loughran added a comment - consider static importing the keys and then refer to them without the prefix
        Hide
        arpitagarwal Arpit Agarwal added a comment -

        Thanks for filing this BELUGA BEHR. A search for io.file.buffer.size shows a large number of files that hard-code this setting name. Do you want to take a crack at fixing these too?

        If not I'd be fine with your current patch as it's an improvement over the status quo. Can you please fix the coding style e.g. lines wider than 80 columns, missing indentation of continued expression at line 734.

        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/BZip2Codec.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/DefaultCodec.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/GzipCodec.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Compression.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/IOUtils.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SequenceFile.java
        hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
        hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
        hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
        hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestTruncatedInputBug.java
        hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileSeqFileComparison.java
        hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSequenceFileSync.java
        hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitLocalRead.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IFile.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestLineRecordReader.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestLineRecordReader.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/DistributedFSCheck.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/IOMapperBase.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/DFSCIOTest.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/JHLogAnalyzer.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/slive/CreateOp.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/TestDFSIO.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/TestFileSystem.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestConcatenatedCompressedInput.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestMRCJCFileInputFormat.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestTextInputFormat.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestMRCJCFileInputFormat.java
        hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestCombineFileInputFormat.java
        hadoop-tools/hadoop-archives/src/main/java/org/apache/hadoop/tools/HadoopArchives.java
        hadoop-tools/hadoop-openstack/src/main/java/org/apache/hadoop/fs/swift/util/SwiftTestUtils.java
        hadoop-tools/hadoop-openstack/src/test/java/org/apache/hadoop/fs/swift/TestSwiftFileSystemPartitionedUploads.java
        
        Show
        arpitagarwal Arpit Agarwal added a comment - Thanks for filing this BELUGA BEHR . A search for io.file.buffer.size shows a large number of files that hard-code this setting name. Do you want to take a crack at fixing these too? If not I'd be fine with your current patch as it's an improvement over the status quo. Can you please fix the coding style e.g. lines wider than 80 columns, missing indentation of continued expression at line 734. hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/BZip2Codec.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/DefaultCodec.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/GzipCodec.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Compression.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/IOUtils.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SequenceFile.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestTruncatedInputBug.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileSeqFileComparison.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSequenceFileSync.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitLocalRead.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/IFile.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestLineRecordReader.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestLineRecordReader.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/DistributedFSCheck.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/IOMapperBase.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/DFSCIOTest.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/JHLogAnalyzer.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/slive/CreateOp.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/TestDFSIO.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/fs/TestFileSystem.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestConcatenatedCompressedInput.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestMRCJCFileInputFormat.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestTextInputFormat.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestMRCJCFileInputFormat.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestCombineFileInputFormat.java hadoop-tools/hadoop-archives/src/main/java/org/apache/hadoop/tools/HadoopArchives.java hadoop-tools/hadoop-openstack/src/main/java/org/apache/hadoop/fs/swift/util/SwiftTestUtils.java hadoop-tools/hadoop-openstack/src/test/java/org/apache/hadoop/fs/swift/TestSwiftFileSystemPartitionedUploads.java

          People

          • Assignee:
            belugabehr BELUGA BEHR
            Reporter:
            belugabehr BELUGA BEHR
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development