Hadoop Common
  1. Hadoop Common
  2. HADOOP-6801

io.sort.mb and io.sort.factor were renamed and moved to mapreduce but are still in CommonConfigurationKeysPublic.java and used in SequenceFile.java

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 0.22.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Target Version/s:

      Description

      Following configuration keys in CommonConfigurationKeysPublic.java (former CommonConfigurationKeys.java):

      public static final String IO_SORT_MB_KEY = "io.sort.mb";
      public static final String IO_SORT_FACTOR_KEY = "io.sort.factor";

      are partially moved:

      • they were renamed to mapreduce.task.io.sort.mb and mapreduce.task.io.sort.factor respectively
      • they were moved to mapreduce project, documented in mapred-default.xml

      However:

      • they are still listed in CommonConfigurationKeysPublic.java as quoted above
      • strings "io.sort.mb" and "io.sort.factor" are used in SequenceFile.java in Hadoop Common project

      Not sure what the solution is, these constants should probably be removed from CommonConfigurationKeysPublic.java but I am not sure what's the best solution for SequenceFile.java.

      1. HADOOP-6801.r1.diff
        3 kB
        Harsh J
      2. HADOOP-6801.r2.diff
        4 kB
        Harsh J
      3. HADOOP-6801.r3.diff
        8 kB
        Harsh J
      4. HADOOP-6801.r4.diff
        8 kB
        Harsh J
      5. HADOOP-6801.r5.diff
        8 kB
        Harsh J
      6. HADOOP-6801.05.patch
        8 kB
        Allen Wittenauer

        Issue Links

          Activity

          Erik Steffl created issue -
          Nigel Daley made changes -
          Field Original Value New Value
          Fix Version/s 0.22.0 [ 12314296 ]
          Hide
          Harsh J added a comment -

          One solution would be to make these options sequence-file specific since they can't obviously be related to MapReduce here?

          Show
          Harsh J added a comment - One solution would be to make these options sequence-file specific since they can't obviously be related to MapReduce here?
          Hide
          Harsh J added a comment -

          Or perhaps add Sorter constructors that let one specify, and fall back to a documented default instead of a configuration carried one.

          Show
          Harsh J added a comment - Or perhaps add Sorter constructors that let one specify, and fall back to a documented default instead of a configuration carried one.
          Harsh J made changes -
          Assignee Harsh J [ qwertymaniac ]
          Harsh J made changes -
          Link This issue incorporates MAPREDUCE-2622 [ MAPREDUCE-2622 ]
          Hide
          Harsh J added a comment -

          Patch that introduces two new options over io.sort.mb and io.sort.factor, specifically for SequenceFile.Sorter:

          seq.io.sort.mb = 100
          seq.io.sort.factor = 100
          

          Descriptions included inside core-default.xml changes as well.

          I did not yet write checks for deprecated io.sort.* properties. Is the double deprecation fine here (MR already deprecates this, and now Common has to too, for new SequenceFile.Sorter specific properties)?

          I believe DistCp would need documentation work as well, since it uses SequenceFile.Sorter

          Show
          Harsh J added a comment - Patch that introduces two new options over io.sort.mb and io.sort.factor, specifically for SequenceFile.Sorter: seq.io.sort.mb = 100 seq.io.sort.factor = 100 Descriptions included inside core-default.xml changes as well. I did not yet write checks for deprecated io.sort.* properties. Is the double deprecation fine here (MR already deprecates this, and now Common has to too, for new SequenceFile.Sorter specific properties)? I believe DistCp would need documentation work as well, since it uses SequenceFile.Sorter
          Harsh J made changes -
          Attachment HADOOP-6801.r1.diff [ 12483816 ]
          Harsh J made changes -
          Link This issue incorporates MAPREDUCE-2703 [ MAPREDUCE-2703 ]
          Hide
          Harsh J added a comment -

          Updating patch for trunk.

          Show
          Harsh J added a comment - Updating patch for trunk.
          Harsh J made changes -
          Attachment HADOOP-6801.r2.diff [ 12507832 ]
          Harsh J made changes -
          Target Version/s 0.24.0 [ 12317652 ]
          Harsh J made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Harsh J added a comment -

          Hey folks, can someone pitch in and give this mostly-docfix a quick review?

          Show
          Harsh J added a comment - Hey folks, can someone pitch in and give this mostly-docfix a quick review?
          Hide
          Harsh J added a comment -

          Updated patch.

          • Improved docs.
          • Added more javadoc links to right places.
          • Added deprecated and new property tests for graceful change.
          Show
          Harsh J added a comment - Updated patch. Improved docs. Added more javadoc links to right places. Added deprecated and new property tests for graceful change.
          Harsh J made changes -
          Attachment HADOOP-6801.r3.diff [ 12526628 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12526628/HADOOP-6801.r3.diff
          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 generated 1938 javac compiler warnings (more than the trunk's current 1934 warnings).

          -1 javadoc. The javadoc tool appears to have generated 8 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/989//testReport/
          Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/989//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/989//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/12526628/HADOOP-6801.r3.diff 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 generated 1938 javac compiler warnings (more than the trunk's current 1934 warnings). -1 javadoc. The javadoc tool appears to have generated 8 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/989//testReport/ Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/989//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/989//console This message is automatically generated.
          Hide
          Harsh J added a comment -

          The javac warning was cause of the graceful config deprecation code added, which referred to deprecated keys a few times. I've suppressed that particular constructor to not warn for now in this new patch.

          Show
          Harsh J added a comment - The javac warning was cause of the graceful config deprecation code added, which referred to deprecated keys a few times. I've suppressed that particular constructor to not warn for now in this new patch.
          Harsh J made changes -
          Attachment HADOOP-6801.r4.diff [ 12526631 ]
          Hide
          Harsh J added a comment -

          The javac warnings further, were cause of a bad link to SequenceFile.Sorter (inner class)

          [WARNING] /Users/harshchouraria/Work/code/apache/hadoop/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java:142: warning - Tag @link: reference not found: SequenceFile.Sorter

          In this new patch I've fixed the javac warns and javadoc warns both.

          Show
          Harsh J added a comment - The javac warnings further, were cause of a bad link to SequenceFile.Sorter (inner class) [WARNING] /Users/harshchouraria/Work/code/apache/hadoop/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java:142: warning - Tag @link: reference not found: SequenceFile.Sorter In this new patch I've fixed the javac warns and javadoc warns both.
          Harsh J made changes -
          Attachment HADOOP-6801.r5.diff [ 12526633 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12526631/HADOOP-6801.r4.diff
          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 appears to have generated 8 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.fs.viewfs.TestViewFsTrash

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/990//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/990//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/12526631/HADOOP-6801.r4.diff 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 appears to have generated 8 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.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/990//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/990//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12526633/HADOOP-6801.r5.diff
          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.fs.viewfs.TestViewFsTrash

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/991//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/991//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/12526633/HADOOP-6801.r5.diff 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.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/991//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/991//console This message is automatically generated.
          Harsh J made changes -
          Target Version/s 0.24.0 [ 12317652 ] 3.0.0 [ 12320357 ]
          Hide
          Harsh J added a comment -

          Patch still applies to trunk. Any refinements needed?

          Here's what the patch does:

          1. Introduces two new properties: seq.io.sort.mb and seq.io.sort.factor that apply to SequenceFile.Sorter classes alone, as replacements to the now-MR properties of io.sort.mb and io.sort.factor.
          2. Makes the SequenceFile.Sorter read these new properties, while paying more importance to the older names.
          3. Docs and read-tests on the new properties.

          Show
          Harsh J added a comment - Patch still applies to trunk. Any refinements needed? Here's what the patch does: 1. Introduces two new properties: seq.io.sort.mb and seq.io.sort.factor that apply to SequenceFile.Sorter classes alone, as replacements to the now-MR properties of io.sort.mb and io.sort.factor . 2. Makes the SequenceFile.Sorter read these new properties, while paying more importance to the older names. 3. Docs and read-tests on the new properties.
          Allen Wittenauer made changes -
          Attachment HADOOP-6801.05.patch [ 12729958 ]
          Hide
          Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 15m 9s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
          +1 javac 7m 41s There were no new javac warning messages.
          +1 javadoc 9m 54s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 5s The applied patch generated 6 new checkstyle issues (total was 450, now 455).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 35s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 1m 43s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
          +1 common tests 23m 53s Tests passed in hadoop-common.
              62m 1s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12729958/HADOOP-6801.05.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 6ae2a0d
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/artifact/patchprocess/diffcheckstylehadoop-common.txt
          hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/artifact/patchprocess/testrun_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/testReport/
          Java 1.7.0_55
          uname Linux asf904.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/console

          This message was automatically generated.

          Show
          Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 9s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 41s There were no new javac warning messages. +1 javadoc 9m 54s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 5s The applied patch generated 6 new checkstyle issues (total was 450, now 455). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 43s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 common tests 23m 53s Tests passed in hadoop-common.     62m 1s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12729958/HADOOP-6801.05.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 6ae2a0d checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/artifact/patchprocess/diffcheckstylehadoop-common.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/testReport/ Java 1.7.0_55 uname Linux asf904.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6419/console This message was automatically generated.
          Allen Wittenauer made changes -
          Labels BB2015-05-TBR
          Ray Chiang made changes -
          Labels BB2015-05-TBR
          Ray Chiang made changes -
          Labels BB2015-05-RFC
          Chris Nauroth made changes -
          Labels BB2015-05-RFC
          Hide
          Chris Nauroth added a comment -

          Hello Harsh J. This patch looks good. The checkstyle warnings in the last Jenkins run are unrelated. Do you also want to add a test that sets both the deprecated and the new properties with different values, and then asserts that the deprecated properties are the ones that take precedence?

          Show
          Chris Nauroth added a comment - Hello Harsh J . This patch looks good. The checkstyle warnings in the last Jenkins run are unrelated. Do you also want to add a test that sets both the deprecated and the new properties with different values, and then asserts that the deprecated properties are the ones that take precedence?
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          563d 12h 27m 1 Harsh J 18/Dec/11 10:51

            People

            • Assignee:
              Harsh J
              Reporter:
              Erik Steffl
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development