Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-3302

Remove the last dependency call from org.apache.hadoop.record package in MR.

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha1
    • Fix Version/s: 3.0.0-alpha1
    • Component/s: client
    • Labels:
      None

      Description

      SecureShuffleUtils provides the following helper:

        /**
         * verify that hash equals to HMacHash(msg)
         * @param newHash
         * @return true if is the same
         */
        private static boolean verifyHash(byte[] hash, byte[] msg, SecretKey key) {
          byte[] msg_hash = generateByteHash(msg, key);
          return Utils.compareBytes(msg_hash, 0, msg_hash.length, hash, 0, hash.length) == 0;
        }
      

      The Utils class used there is org.apache.hadoop.record.Utils. With the record common package going away via HADOOP-7781, the internal (and also deprecated on the whole) compareBytes utility must be moved elsewhere.

      The Utils#compareBytes contains:

      /** Lexicographic order of binary data. */
        public static int compareBytes(byte[] b1, int s1, int l1,
                                       byte[] b2, int s2, int l2) {
          return WritableComparator.compareBytes(b1, s1, l1, b2, s2, l2);
        }
      

      Which looks like it can be replaced inline, as it appears to be a dummy wrapper call. I'll put up a patch with this inline replacement shortly for review.

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          -1 tests included. 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 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 passed unit tests in .

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

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12501270/MAPREDUCE-3302.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. 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 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 passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1190//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1190//console This message is automatically generated.
          Hide
          qwertymaniac Harsh J added a comment -
          -1 tests included. 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.
          

          The patch provides mere inlining of code to remove a deprecated function call, since the deprecated package is going away. WritableComparator#compareBytes is otherwise well tested already by existing tests.

          Hence, no test case is specifically required for this change.

          Show
          qwertymaniac Harsh J added a comment - -1 tests included. 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. The patch provides mere inlining of code to remove a deprecated function call, since the deprecated package is going away. WritableComparator#compareBytes is otherwise well tested already by existing tests. Hence, no test case is specifically required for this change.
          Hide
          qwertymaniac Harsh J added a comment -

          Per parent JIRA, recordio is still in use apparently. Resolving as won't fix.

          Show
          qwertymaniac Harsh J added a comment - Per parent JIRA, recordio is still in use apparently. Resolving as won't fix.
          Hide
          qwertymaniac Harsh J added a comment -

          Actually, this can and should go in anyway. Its the only one using the wrong API.

          Show
          qwertymaniac Harsh J added a comment - Actually, this can and should go in anyway. Its the only one using the wrong API.
          Hide
          qwertymaniac Harsh J added a comment -

          I consider this a trivial change as I've already noted above that the implementation call we're switching to has the exact same definition as the former. If no one has further comments, I'll commit this in by Monday.

          Show
          qwertymaniac Harsh J added a comment - I consider this a trivial change as I've already noted above that the implementation call we're switching to has the exact same definition as the former. If no one has further comments, I'll commit this in by Monday.
          Hide
          qwertymaniac Harsh J added a comment -

          Changing type to Improvement (earlier: Task).

          Show
          qwertymaniac Harsh J added a comment - Changing type to Improvement (earlier: Task).
          Hide
          qwertymaniac Harsh J added a comment -

          Committed revision 1342613 to trunk.

          Show
          qwertymaniac Harsh J added a comment - Committed revision 1342613 to trunk.
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2361 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2361/)
          MAPREDUCE-3302. Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613)

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2361 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2361/ ) MAPREDUCE-3302 . Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342613 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2288 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2288/)
          MAPREDUCE-3302. Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613)

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2288 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2288/ ) MAPREDUCE-3302 . Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342613 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2307 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2307/)
          MAPREDUCE-3302. Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613)

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2307 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2307/ ) MAPREDUCE-3302 . Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342613 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1057 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1057/)
          MAPREDUCE-3302. Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613)

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1057 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1057/ ) MAPREDUCE-3302 . Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342613 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1091 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1091/)
          MAPREDUCE-3302. Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613)

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1091 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1091/ ) MAPREDUCE-3302 . Remove the last dependency call from org.apache.hadoop.record package in MR. (harsh) (Revision 1342613) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342613 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/security/SecureShuffleUtils.java

            People

            • Assignee:
              qwertymaniac Harsh J
              Reporter:
              qwertymaniac Harsh J
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development