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.05.patch
        8 kB
        Allen Wittenauer
      2. HADOOP-6801.r1.diff
        3 kB
        Harsh J
      3. HADOOP-6801.r2.diff
        4 kB
        Harsh J
      4. HADOOP-6801.r3.diff
        8 kB
        Harsh J
      5. HADOOP-6801.r4.diff
        8 kB
        Harsh J
      6. HADOOP-6801.r5.diff
        8 kB
        Harsh J

        Issue Links

          Activity

          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?
          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.
          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.
          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.
          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
          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.
          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.
          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 -

          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.
          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 -

          Updating patch for trunk.

          Show
          Harsh J added a comment - Updating patch for trunk.
          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
          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.
          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?

            People

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

              Dates

              • Created:
                Updated:

                Development