Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-10576 DiskBalancer followup work items
  3. HDFS-10871

DiskBalancerWorkItem should not import jackson relocated by htrace

Details

    • Sub-task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 3.0.0-alpha1
    • 3.0.0-alpha2
    • hdfs-client
    • None

    Description

      Compiling trunk against upstream htrace fails since it does not bundle the org.apache.htrace.fasterxml.jackson.annotation.JsonInclude.

      Attachments

        1. HDFS-10871.001.patch
          3 kB
          Manoj Govindassamy

        Activity

          [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project hadoop-hdfs-client: Compilation failure: Compilation failure:
          [ERROR] /home/iwasakims/srcs/hadoop/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java:[25,54] package org.apache.htrace.fasterxml.jackson.annotation does not exist
          [ERROR] /home/iwasakims/srcs/hadoop/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java:[36,2] cannot find symbol
          
          iwasakims Masatake Iwasaki added a comment - [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project hadoop-hdfs-client: Compilation failure: Compilation failure: [ERROR] /home/iwasakims/srcs/hadoop/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java:[25,54] package org.apache.htrace.fasterxml.jackson.annotation does not exist [ERROR] /home/iwasakims/srcs/hadoop/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java:[36,2] cannot find symbol

          Taking up this issue after discussing with anu.

          manojg Manoj Govindassamy added a comment - Taking up this issue after discussing with anu .

          I am able to recreate the problem with htrace upstream. Now, below are the fix proposals. Both these fixes seem to solve the problem for the latest upstream htrace.

          1. Change the dependency library

          • Add com.fasterxml.jackson.core as a dependent artifact in hadoop-hdfs-project/hadoop-hdfs-client/ project. Remember, hadoop-hdfs-project/ project is already dependent on com.fasterxml.jackson.core
            • hadoop-hdfs-project/hadoop-hdfs-client/pom.xml
                109     </dependency>
                110       <dependency>                                                                                                                               
                111           <groupId>com.fasterxml.jackson.core</groupId>                                                                                          
                112           <artifactId>jackson-annotations</artifactId>                                                                                           
                113       </dependency>
                114   </dependencies> 
          • In DiskBalancerWorkItem and MoveStep, replace the import org.apache.htrace.fasterxml.jackson with com.fasterxml.jackson
            -import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude;
            +import com.fasterxml.jackson.annotation.JsonInclude;
            
          • No other JsonInclude annotation changes are needed

          2. Make use of org.codehaus.jackson.map.annotate instead of org.apache.htrace.fasterxml.jackson

          • DiskBalancerWorkItem Object Mapper can have an explicit serialization inclusion instead of class level annotation
            diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java
            index 592a89facf16bb3d046e0f87c83f571c2d68443a..edd2801c072968f5a2dc46941e847bd8cba9157a 100644
            --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java
            +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java
            @@ -19,12 +19,13 @@
             
             package org.apache.hadoop.hdfs.server.datanode;
             
            +import com.fasterxml.jackson.annotation.JsonInclude;
             import com.google.common.base.Preconditions;
             import org.apache.hadoop.classification.InterfaceAudience;
             import org.apache.hadoop.classification.InterfaceStability;
            -import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude;
             import org.codehaus.jackson.map.ObjectMapper;
             import org.codehaus.jackson.map.ObjectReader;
            +import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
             
             import java.io.IOException;
             
            @@ -33,7 +34,6 @@
              */
             @InterfaceAudience.Private
             @InterfaceStability.Unstable
            -@JsonInclude(JsonInclude.Include.NON_DEFAULT)
             public class DiskBalancerWorkItem {
               private static final ObjectMapper MAPPER = new ObjectMapper();
               private static final ObjectReader READER =
            @@ -52,6 +52,13 @@
               private long bandwidth;
             
               /**
            +   * Initialization block for static members
            +   */
            +  static {
            +    MAPPER.setSerializationInclusion(Inclusion.NON_DEFAULT);
            +  }
            +
            +  /**
                * Empty constructor for Json serialization.
                */
               public DiskBalancerWorkItem() {
            
          • MoveStep annotation can be removed as there are no Object Mappers for the class. Wondering if from/to json strings is necessary for this class at all ?
            diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java
            index b5f68fd8ad3ee8a803f039c422c23145935e589d..97fd650808d2b3e29c5d8132c755ef9096fc68de 100644
            --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java
            +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java
            @@ -17,20 +17,10 @@
             
             package org.apache.hadoop.hdfs.server.diskbalancer.planner;
             
             import org.apache.hadoop.hdfs.server.diskbalancer.datamodel.DiskBalancerVolume;
             import org.apache.hadoop.util.StringUtils;
            -import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude;
             
            -/**
            - * Ignore fields with default values. In most cases Throughtput, diskErrors
            - * tolerancePercent and bandwidth will be the system defaults.
            - * So we will avoid serializing them into JSON.
            - */
            -@JsonInclude(JsonInclude.Include.NON_DEFAULT)
             /**
              * Move step is a step that planner can execute that will move data from one
              * volume to another.
            

          eddyxu, anu any thoughts on the above proposals ?

          manojg Manoj Govindassamy added a comment - I am able to recreate the problem with htrace upstream. Now, below are the fix proposals. Both these fixes seem to solve the problem for the latest upstream htrace. 1. Change the dependency library Add com.fasterxml.jackson.core as a dependent artifact in hadoop-hdfs-project/hadoop-hdfs-client/ project. Remember, hadoop-hdfs-project/ project is already dependent on com.fasterxml.jackson.core hadoop-hdfs-project/hadoop-hdfs-client/pom.xml 109 </dependency> 110 <dependency> 111 <groupId>com.fasterxml.jackson.core</groupId> 112 <artifactId>jackson-annotations</artifactId> 113 </dependency> 114 </dependencies> In DiskBalancerWorkItem and MoveStep , replace the import org.apache.htrace.fasterxml.jackson with com.fasterxml.jackson - import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude; + import com.fasterxml.jackson.annotation.JsonInclude; No other JsonInclude annotation changes are needed 2. Make use of org.codehaus.jackson.map.annotate instead of org.apache.htrace.fasterxml.jackson DiskBalancerWorkItem Object Mapper can have an explicit serialization inclusion instead of class level annotation diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java index 592a89facf16bb3d046e0f87c83f571c2d68443a..edd2801c072968f5a2dc46941e847bd8cba9157a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java @@ -19,12 +19,13 @@ package org.apache.hadoop.hdfs.server.datanode; + import com.fasterxml.jackson.annotation.JsonInclude; import com.google.common.base.Preconditions; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; - import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude; import org.codehaus.jackson.map.ObjectMapper; import org.codehaus.jackson.map.ObjectReader; + import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion; import java.io.IOException; @@ -33,7 +34,6 @@ */ @InterfaceAudience.Private @InterfaceStability.Unstable -@JsonInclude(JsonInclude.Include.NON_DEFAULT) public class DiskBalancerWorkItem { private static final ObjectMapper MAPPER = new ObjectMapper(); private static final ObjectReader READER = @@ -52,6 +52,13 @@ private long bandwidth; /** + * Initialization block for static members + */ + static { + MAPPER.setSerializationInclusion(Inclusion.NON_DEFAULT); + } + + /** * Empty constructor for Json serialization. */ public DiskBalancerWorkItem() { MoveStep annotation can be removed as there are no Object Mappers for the class. Wondering if from/to json strings is necessary for this class at all ? diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java index b5f68fd8ad3ee8a803f039c422c23145935e589d..97fd650808d2b3e29c5d8132c755ef9096fc68de 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java @@ -17,20 +17,10 @@ package org.apache.hadoop.hdfs.server.diskbalancer.planner; import org.apache.hadoop.hdfs.server.diskbalancer.datamodel.DiskBalancerVolume; import org.apache.hadoop.util.StringUtils; - import org.apache.htrace.fasterxml.jackson.annotation.JsonInclude; -/** - * Ignore fields with default values. In most cases Throughtput, diskErrors - * tolerancePercent and bandwidth will be the system defaults. - * So we will avoid serializing them into JSON. - */ -@JsonInclude(JsonInclude.Include.NON_DEFAULT) /** * Move step is a step that planner can execute that will move data from one * volume to another. eddyxu , anu any thoughts on the above proposals ?
          aengineer Anu Engineer added a comment -

          Thanks for taking care of this issue. I like proposal 1. Since It is cleaner and we will need to share json classes between client and server anyway.

          aengineer Anu Engineer added a comment - Thanks for taking care of this issue. I like proposal 1. Since It is cleaner and we will need to share json classes between client and server anyway.

          Attcached v01 patch.

          • Added com.fasterxml.jackson.core as a dependent artifact in hadoop-hdfs-project/hadoop-hdfs-client/ project
          • Updated DiskBalancerWorkItem and MoveStep to use the right import.

          Testing:

          • Updated hadoop's hadoop-project/pom.xml to include the latest upstream htrace artifacts (4.3.0-incubating-SNAPSHOT) as dependency
          • Verified top level mvn install for upstream hadoop
          • Verified mvn test TestDiskBalancer*
          manojg Manoj Govindassamy added a comment - Attcached v01 patch. Added com.fasterxml.jackson.core as a dependent artifact in hadoop-hdfs-project/hadoop-hdfs-client/ project Updated DiskBalancerWorkItem and MoveStep to use the right import. Testing: Updated hadoop's hadoop-project/pom.xml to include the latest upstream htrace artifacts (4.3.0-incubating-SNAPSHOT) as dependency Verified top level mvn install for upstream hadoop Verified mvn test TestDiskBalancer*
          aengineer Anu Engineer added a comment -

          manojg Thanks for fixing this so quickly. +1, pending Jenkins.

          aengineer Anu Engineer added a comment - manojg Thanks for fixing this so quickly. +1, pending Jenkins.
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s 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.
          0 mvndep 0m 6s Maven dependency ordering for branch
          +1 mvninstall 7m 48s trunk passed
          +1 compile 1m 46s trunk passed
          +1 checkstyle 0m 34s trunk passed
          +1 mvnsite 1m 35s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 27s trunk passed
          +1 javadoc 1m 18s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 30s the patch passed
          +1 compile 1m 31s the patch passed
          +1 javac 1m 31s the patch passed
          +1 checkstyle 0m 32s the patch passed
          +1 mvnsite 1m 33s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 2s The patch has no ill-formed XML file.
          +1 findbugs 3m 35s the patch passed
          +1 javadoc 1m 19s the patch passed
          +1 unit 0m 56s hadoop-hdfs-client in the patch passed.
          +1 unit 60m 20s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          90m 51s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10871
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829294/HDFS-10871.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 0e565a8ec9d7 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 98bdb51
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16800/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16800/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s 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. 0 mvndep 0m 6s Maven dependency ordering for branch +1 mvninstall 7m 48s trunk passed +1 compile 1m 46s trunk passed +1 checkstyle 0m 34s trunk passed +1 mvnsite 1m 35s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 27s trunk passed +1 javadoc 1m 18s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 30s the patch passed +1 compile 1m 31s the patch passed +1 javac 1m 31s the patch passed +1 checkstyle 0m 32s the patch passed +1 mvnsite 1m 33s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 3m 35s the patch passed +1 javadoc 1m 19s the patch passed +1 unit 0m 56s hadoop-hdfs-client in the patch passed. +1 unit 60m 20s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 90m 51s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10871 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829294/HDFS-10871.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 0e565a8ec9d7 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 98bdb51 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16800/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16800/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

          Thanks for the review anu. Manually verified the fix against latest upstream htrace.

          manojg Manoj Govindassamy added a comment - Thanks for the review anu . Manually verified the fix against latest upstream htrace.
          aengineer Anu Engineer added a comment -

          manojg Thanks for fixing this issue. I will commit this shortly

          aengineer Anu Engineer added a comment - manojg Thanks for fixing this issue. I will commit this shortly
          aengineer Anu Engineer added a comment -

          manojg Thanks for the contribution. I have committed this to trunk.

          aengineer Anu Engineer added a comment - manojg Thanks for the contribution. I have committed this to trunk.
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10477 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10477/)
          HDFS-10871. DiskBalancerWorkItem should not import jackson relocated by (aengineer: rev d85d9b2e7b4df04335111f4e3ce21a2fda39aee9)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/pom.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10477 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10477/ ) HDFS-10871 . DiskBalancerWorkItem should not import jackson relocated by (aengineer: rev d85d9b2e7b4df04335111f4e3ce21a2fda39aee9) (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/MoveStep.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/pom.xml (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/datanode/DiskBalancerWorkItem.java

          People

            manojg Manoj Govindassamy
            iwasakims Masatake Iwasaki
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: