Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-10556

DistCpOptions should be validated automatically

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0
    • Component/s: distcp
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      DistCpOptions can be set from command-line or may be set manually. In HDFS-10397, we refactored the validation to make it simpler and more efficient. However, the newly added validate() method may not be automatically revoked. This is the major concern for existing downstreams that create the DistCpOptions manually instead of by parser, and have conflicting options.

      This jira is to make the validation happen automatically. A simple fix is to use the approach that validates in individual setters. This is a fix for branch-2. As a long-term fix, in HDFS-10533, we're making the DistCpOptions immutable so that it will be hard, if not impossible, to use it wrongly by downstream applications. However, that code will only go to trunk branch as it breaks backwards-compatibility.

      1. HDFS-10556-branch-2.000.patch
        25 kB
        Mingliang Liu
      2. HDFS-10556-branch-2.001.patch
        28 kB
        Mingliang Liu

        Issue Links

          Activity

          Hide
          liuml07 Mingliang Liu added a comment - - edited

          The v0 patch simply calls the validate() private method in setters of the dependent (or exclusive) options. The basic idea is kinda similar to the code before HDFS-10397. It also adds a new unit test to test constructing DistCpOptions manually with setters. Its test cases are very similar to the parser test in TestOptionsParser, except those who are validated in parser instead of the DistCpOptions#validate().

          Show
          liuml07 Mingliang Liu added a comment - - edited The v0 patch simply calls the validate() private method in setters of the dependent (or exclusive) options. The basic idea is kinda similar to the code before HDFS-10397 . It also adds a new unit test to test constructing DistCpOptions manually with setters. Its test cases are very similar to the parser test in TestOptionsParser , except those who are validated in parser instead of the DistCpOptions#validate() .
          Hide
          liuml07 Mingliang Liu added a comment -

          Per-offline discussion with Jing Zhao, we should keep the validate() method signature as it-was. The reason was that it's a public API and downstream applications may call it directly before calling setters. The v1 patch is to make the code similar to pre- HDFS-10397. The changes are:

          1. Ignore the delete option if useDiff is set. This is processed in both setDeleteMissing() and setUseDiff()
          2. Reuse the DistCpOptions from v0 patch for unit test
          Show
          liuml07 Mingliang Liu added a comment - Per-offline discussion with Jing Zhao , we should keep the validate() method signature as it-was. The reason was that it's a public API and downstream applications may call it directly before calling setters. The v1 patch is to make the code similar to pre- HDFS-10397 . The changes are: Ignore the delete option if useDiff is set. This is processed in both setDeleteMissing() and setUseDiff() Reuse the DistCpOptions from v0 patch for unit test
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 11m 21s Docker mode activated.
          +1 @author 0m 1s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 8m 50s branch-2 passed
          +1 compile 0m 18s branch-2 passed with JDK v1.8.0_91
          +1 compile 0m 20s branch-2 passed with JDK v1.7.0_101
          +1 checkstyle 0m 18s branch-2 passed
          +1 mvnsite 0m 28s branch-2 passed
          +1 mvneclipse 0m 47s branch-2 passed
          +1 findbugs 0m 40s branch-2 passed
          +1 javadoc 0m 14s branch-2 passed with JDK v1.8.0_91
          +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_101
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 14s the patch passed with JDK v1.8.0_91
          +1 javac 0m 14s the patch passed
          +1 compile 0m 17s the patch passed with JDK v1.7.0_101
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 14s hadoop-tools/hadoop-distcp: The patch generated 23 new + 79 unchanged - 0 fixed = 102 total (was 79)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 42s the patch passed
          +1 javadoc 0m 9s the patch passed with JDK v1.8.0_91
          +1 javadoc 0m 12s the patch passed with JDK v1.7.0_101
          +1 unit 7m 17s hadoop-distcp in the patch passed with JDK v1.7.0_101.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          43m 10s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:d1c475d
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812607/HDFS-10556-branch-2.001.patch
          JIRA Issue HDFS-10556
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 74973bcca4cb 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 81497a4
          Default Java 1.7.0_101
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15878/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-distcp.txt
          JDK v1.7.0_101 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15878/testReport/
          modules C: hadoop-tools/hadoop-distcp U: hadoop-tools/hadoop-distcp
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15878/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 11m 21s Docker mode activated. +1 @author 0m 1s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 8m 50s branch-2 passed +1 compile 0m 18s branch-2 passed with JDK v1.8.0_91 +1 compile 0m 20s branch-2 passed with JDK v1.7.0_101 +1 checkstyle 0m 18s branch-2 passed +1 mvnsite 0m 28s branch-2 passed +1 mvneclipse 0m 47s branch-2 passed +1 findbugs 0m 40s branch-2 passed +1 javadoc 0m 14s branch-2 passed with JDK v1.8.0_91 +1 javadoc 0m 16s branch-2 passed with JDK v1.7.0_101 +1 mvninstall 0m 19s the patch passed +1 compile 0m 14s the patch passed with JDK v1.8.0_91 +1 javac 0m 14s the patch passed +1 compile 0m 17s the patch passed with JDK v1.7.0_101 +1 javac 0m 17s the patch passed -0 checkstyle 0m 14s hadoop-tools/hadoop-distcp: The patch generated 23 new + 79 unchanged - 0 fixed = 102 total (was 79) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 42s the patch passed +1 javadoc 0m 9s the patch passed with JDK v1.8.0_91 +1 javadoc 0m 12s the patch passed with JDK v1.7.0_101 +1 unit 7m 17s hadoop-distcp in the patch passed with JDK v1.7.0_101. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 43m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:d1c475d JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812607/HDFS-10556-branch-2.001.patch JIRA Issue HDFS-10556 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 74973bcca4cb 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 81497a4 Default Java 1.7.0_101 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15878/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-distcp.txt JDK v1.7.0_101 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15878/testReport/ modules C: hadoop-tools/hadoop-distcp U: hadoop-tools/hadoop-distcp Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15878/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingzhao Jing Zhao added a comment -

          Thanks for the fix, Mingliang. The patch looks good to me. +1.

          Show
          jingzhao Jing Zhao added a comment - Thanks for the fix, Mingliang. The patch looks good to me. +1.
          Hide
          jingzhao Jing Zhao added a comment -

          I've committed this into branch-2 and branch-2.8.

          Show
          jingzhao Jing Zhao added a comment - I've committed this into branch-2 and branch-2.8.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Jing Zhao for the review and commit!

          Show
          liuml07 Mingliang Liu added a comment - Thanks Jing Zhao for the review and commit!

            People

            • Assignee:
              liuml07 Mingliang Liu
              Reporter:
              liuml07 Mingliang Liu
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development