Hadoop Common
  1. Hadoop Common
  2. HADOOP-7493

[HDFS-362] Provide ShortWritable class in hadoop.

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: io
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      As part of HDFS-362, Provide the ShortWritable class.

      1. HADOOP-7493.3_root.patch
        6 kB
        Tsz Wo Nicholas Sze
      2. HADOOP-7493.3.patch
        6 kB
        Uma Maheswara Rao G
      3. HADOOP-7953.2.patch
        7 kB
        Uma Maheswara Rao G
      4. HADOOP-7493.1.patch
        4 kB
        Uma Maheswara Rao G

        Issue Links

          Activity

          Hide
          Uma Maheswara Rao G added a comment -

          This patch will be tested along with HDFS-362.

          Show
          Uma Maheswara Rao G added a comment - This patch will be tested along with HDFS-362 .
          Hide
          Uma Maheswara Rao G added a comment -

          Updated patch against trunk.

          Show
          Uma Maheswara Rao G added a comment - Updated patch against trunk.
          Hide
          Eli Collins added a comment -

          This patch doesn't have any test coverage, eg in TestWritable.

          Show
          Eli Collins added a comment - This patch doesn't have any test coverage, eg in TestWritable.
          Hide
          Uma Maheswara Rao G added a comment -

          Hi Eli,

          Thanks a lot for taking a look!

          Initially why i did not provided separate tests here is, HDFS-362 will ShortWritable completely, So, actual testing will happen through that.

          I agree with you.
          To commit it separately, we must have separate tests. I have provided the tests separately in Common itself along with HADOOP-7953.2.patch now.

          --Thanks

          Show
          Uma Maheswara Rao G added a comment - Hi Eli, Thanks a lot for taking a look! Initially why i did not provided separate tests here is, HDFS-362 will ShortWritable completely, So, actual testing will happen through that. I agree with you. To commit it separately, we must have separate tests. I have provided the tests separately in Common itself along with HADOOP-7953 .2.patch now. --Thanks
          Hide
          Uma Maheswara Rao G added a comment -

          test-patch results are:

          Checking the integrity of system test framework code.
          ======================================================================
          ====================================================================
          +1 overall.
          +1 @author. The patch does not contain any @author tags.

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

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

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

          +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 system test framework. The patch passed system test framework compile.

          ======================================================================
          ======================================================================
          Finished build.
          ======================================================================
          ====================================================================

          Verified the tests as well, there is no test failures because of this change.

          Show
          Uma Maheswara Rao G added a comment - test-patch results are: Checking the integrity of system test framework code. ====================================================================== ==================================================================== +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +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 system test framework. The patch passed system test framework compile. ====================================================================== ====================================================================== Finished build. ====================================================================== ==================================================================== Verified the tests as well, there is no test failures because of this change.
          Hide
          Tsz Wo Nicholas Sze added a comment -
          • There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized
          • Why converting shorts to ints?
            +      int thisValue = (short) readUnsignedShort(b1, s1);
            +      int thatValue = (short) readUnsignedShort(b2, s2);
            
          • Please add @Override tags.
          • There are many tailing spaces in the license header. Please remove them.
          Show
          Tsz Wo Nicholas Sze added a comment - There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized Why converting shorts to ints? + int thisValue = ( short ) readUnsignedShort(b1, s1); + int thatValue = ( short ) readUnsignedShort(b2, s2); Please add @Override tags. There are many tailing spaces in the license header. Please remove them.
          Hide
          Uma Maheswara Rao G added a comment -

          Hi Nicholas,

          Thanks a lot for taking a look and also thanks for the good comments.

          There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized

          Fixed.
          Javadoc example class need to correct in WritableComparable interface.
          I am planning to file other issue for correcting all other writables. What do you say?

          Why converting shorts to ints?
          + int thisValue = (short) readUnsignedShort(b1, s1);
          + int thatValue = (short) readUnsignedShort(b2, s2);

          Anyway return type will be converted to int right. So, i put them as ints.
          Changed to shorts

          Please add @Override tags.

          done.

          There are many tailing spaces in the license header. Please remove them.
          corrected.

          My observation is that, hadoop coding formatter is not removing the training spaces.
          Is it the same case for you? if yes, shall we file one JIRA for correcting this?
          Because i copied that header from some other class (i dont remember exactly), looks that file also might have the same. If we have capture this rule in coding formatter itself, we will not miss this kind of small style issues.

          --Thanks

          Show
          Uma Maheswara Rao G added a comment - Hi Nicholas, Thanks a lot for taking a look and also thanks for the good comments. There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized Fixed. Javadoc example class need to correct in WritableComparable interface. I am planning to file other issue for correcting all other writables. What do you say? Why converting shorts to ints? + int thisValue = (short) readUnsignedShort(b1, s1); + int thatValue = (short) readUnsignedShort(b2, s2); Anyway return type will be converted to int right. So, i put them as ints. Changed to shorts Please add @Override tags. done. There are many tailing spaces in the license header. Please remove them. corrected. My observation is that, hadoop coding formatter is not removing the training spaces. Is it the same case for you? if yes, shall we file one JIRA for correcting this? Because i copied that header from some other class (i dont remember exactly), looks that file also might have the same. If we have capture this rule in coding formatter itself, we will not miss this kind of small style issues. --Thanks
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/42//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/12490630/HADOOP-7493.3.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/42//console This message is automatically generated.
          Hide
          Uma Maheswara Rao G added a comment -

          Manually i can applied the patch successfully by using svn option.

          After that i ran the testpatch commands

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

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

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

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

          +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 system test framework. The patch passed system test framework compile.

          Looks some issue with hudson.

          Also there is not failures because of this patch.

          Show
          Uma Maheswara Rao G added a comment - Manually i can applied the patch successfully by using svn option. After that i ran the testpatch commands +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +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 system test framework. The patch passed system test framework compile. Looks some issue with hudson. Also there is not failures because of this patch.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Seems that Jenkins expects the patch to be generated from the root. Let's give a try.

          HADOOP-7493.3b.patch: same patch generated from root.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good. Seems that Jenkins expects the patch to be generated from the root. Let's give a try. HADOOP-7493 .3b.patch: same patch generated from root.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12490647/HADOOP-7493.3_root.patch
          against trunk revision .

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

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

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

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

          +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 core unit tests:

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//artifact/trunk/target/newPatchFindbugsWarningshadoop-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//artifact/trunk/target/newPatchFindbugsWarningshadoop-annotations.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//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/12490647/HADOOP-7493.3_root.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +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 core unit tests: +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//artifact/trunk/target/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//artifact/trunk/target/newPatchFindbugsWarningshadoop-annotations.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/43//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/12490647/HADOOP-7493.3_root.patch
          against trunk revision .

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

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

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

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

          -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

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

          -1 core tests. The patch failed these core unit tests:

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/44//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/44//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/12490647/HADOOP-7493.3_root.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these core unit tests: +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/44//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/44//console This message is automatically generated.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          According to the console output, the core tests were not executed.

          There are no tests to run.
          
          Results :
          
          Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
          

          However, I was able to run tests locally.

          Results :
          
          Tests run: 1337, Failures: 0, Errors: 0, Skipped: 0
          
          Show
          Tsz Wo Nicholas Sze added a comment - According to the console output , the core tests were not executed. There are no tests to run. Results : Tests run: 0, Failures: 0, Errors: 0, Skipped: 0 However, I was able to run tests locally. Results : Tests run: 1337, Failures: 0, Errors: 0, Skipped: 0
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Uma!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Uma!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #747 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/747/)
          HADOOP-7493. Add ShortWritable. Constributed by Uma Maheswara Rao G

          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158733
          Files :

          • /hadoop/common/trunk/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritable.java
          • /hadoop/common/trunk/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common/src/main/java/org/apache/hadoop/io/ShortWritable.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #747 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/747/ ) HADOOP-7493 . Add ShortWritable. Constributed by Uma Maheswara Rao G szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158733 Files : /hadoop/common/trunk/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritable.java /hadoop/common/trunk/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common/src/main/java/org/apache/hadoop/io/ShortWritable.java
          Hide
          Uma Maheswara Rao G added a comment -

          Thanks Nicholas,

          Regarding the below comment

          There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized

          shall i file separate Jira to fix in other writables as well?

          Show
          Uma Maheswara Rao G added a comment - Thanks Nicholas, Regarding the below comment There is a warning: WritableComparable is a raw type. References to generic type WritableComparable<T> should be parameterized shall i file separate Jira to fix in other writables as well?
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Sure, please file a JIRA for it.

          Show
          Tsz Wo Nicholas Sze added a comment - Sure, please file a JIRA for it.
          Hide
          Uma Maheswara Rao G added a comment -

          filed the JIRA HADOOP-7547

          --Thanks

          Show
          Uma Maheswara Rao G added a comment - filed the JIRA HADOOP-7547 --Thanks

            People

            • Assignee:
              Uma Maheswara Rao G
              Reporter:
              Uma Maheswara Rao G
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development