hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ji...@apache.org
Subject hadoop git commit: HDFS-10397. Distcp should ignore -delete option if -diff option is provided instead of exiting. Contributed by Mingliang Liu.
Date Tue, 17 May 2016 22:46:49 GMT
Repository: hadoop
Updated Branches:
  refs/heads/trunk 947848484 -> 03788d301


HDFS-10397. Distcp should ignore -delete option if -diff option is provided instead of exiting.
Contributed by Mingliang Liu.


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/03788d30
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/03788d30
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/03788d30

Branch: refs/heads/trunk
Commit: 03788d3015c962eac1a35fa5df39356e8b84731c
Parents: 9478484
Author: Jing Zhao <jing9@apache.org>
Authored: Tue May 17 15:44:07 2016 -0700
Committer: Jing Zhao <jing9@apache.org>
Committed: Tue May 17 15:46:30 2016 -0700

----------------------------------------------------------------------
 .../org/apache/hadoop/tools/DistCpOptions.java  | 32 ++++---------
 .../org/apache/hadoop/tools/OptionsParser.java  | 49 +++++++++-----------
 .../apache/hadoop/tools/TestOptionsParser.java  | 19 +++++++-
 3 files changed, 49 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/03788d30/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
index 0b1234b..1bc65e0 100644
--- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
+++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
@@ -160,7 +160,6 @@ public class DistCpOptions {
    * @param atomicCommit - boolean switch
    */
   public void setAtomicCommit(boolean atomicCommit) {
-    validate(DistCpOptionSwitch.ATOMIC_COMMIT, atomicCommit);
     this.atomicCommit = atomicCommit;
   }
 
@@ -179,7 +178,6 @@ public class DistCpOptions {
    * @param syncFolder - boolean switch
    */
   public void setSyncFolder(boolean syncFolder) {
-    validate(DistCpOptionSwitch.SYNC_FOLDERS, syncFolder);
     this.syncFolder = syncFolder;
   }
 
@@ -198,7 +196,6 @@ public class DistCpOptions {
    * @param deleteMissing - boolean switch
    */
   public void setDeleteMissing(boolean deleteMissing) {
-    validate(DistCpOptionSwitch.DELETE_MISSING, deleteMissing);
     this.deleteMissing = deleteMissing;
   }
 
@@ -254,7 +251,6 @@ public class DistCpOptions {
    * @param overwrite - boolean switch
    */
   public void setOverwrite(boolean overwrite) {
-    validate(DistCpOptionSwitch.OVERWRITE, overwrite);
     this.overwrite = overwrite;
   }
 
@@ -270,7 +266,6 @@ public class DistCpOptions {
    * update option and CRC is not skipped.
    */
   public void setAppend(boolean append) {
-    validate(DistCpOptionSwitch.APPEND, append);
     this.append = append;
   }
 
@@ -287,7 +282,6 @@ public class DistCpOptions {
   }
 
   public void setUseDiff(boolean useDiff, String fromSnapshot, String toSnapshot) {
-    validate(DistCpOptionSwitch.DIFF, useDiff);
     this.useDiff = useDiff;
     this.fromSnapshot = fromSnapshot;
     this.toSnapshot = toSnapshot;
@@ -314,7 +308,6 @@ public class DistCpOptions {
    * @param skipCRC - boolean switch
    */
   public void setSkipCRC(boolean skipCRC) {
-    validate(DistCpOptionSwitch.SKIP_CRC, skipCRC);
     this.skipCRC = skipCRC;
   }
 
@@ -551,20 +544,15 @@ public class DistCpOptions {
     this.filtersFile = filtersFilename;
   }
 
-  public void validate(DistCpOptionSwitch option, boolean value) {
-
-    boolean syncFolder = (option == DistCpOptionSwitch.SYNC_FOLDERS ?
-        value : this.syncFolder);
-    boolean overwrite = (option == DistCpOptionSwitch.OVERWRITE ?
-        value : this.overwrite);
-    boolean deleteMissing = (option == DistCpOptionSwitch.DELETE_MISSING ?
-        value : this.deleteMissing);
-    boolean atomicCommit = (option == DistCpOptionSwitch.ATOMIC_COMMIT ?
-        value : this.atomicCommit);
-    boolean skipCRC = (option == DistCpOptionSwitch.SKIP_CRC ?
-        value : this.skipCRC);
-    boolean append = (option == DistCpOptionSwitch.APPEND ? value : this.append);
-    boolean useDiff = (option == DistCpOptionSwitch.DIFF ? value : this.useDiff);
+  void validate() {
+    if (useDiff && deleteMissing) {
+      // -delete and -diff are mutually exclusive. For backward compatibility,
+      // we ignore the -delete option here, instead of throwing an
+      // IllegalArgumentException. See HDFS-10397 for more discussion.
+      OptionsParser.LOG.warn("-delete and -diff are mutually exclusive. " +
+          "The -delete option will be ignored.");
+      setDeleteMissing(false);
+    }
 
     if (syncFolder && atomicCommit) {
       throw new IllegalArgumentException("Atomic commit can't be used with " +
@@ -593,7 +581,7 @@ public class DistCpOptions {
       throw new IllegalArgumentException(
           "Append is disallowed when skipping CRC");
     }
-    if ((!syncFolder || deleteMissing) && useDiff) {
+    if (!syncFolder && useDiff) {
       throw new IllegalArgumentException(
           "Diff is valid only with update options");
     }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/03788d30/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
index 0f0ef2e..5eaf4da 100644
--- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
+++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
@@ -41,7 +41,7 @@ import com.google.common.base.Preconditions;
  */
 public class OptionsParser {
 
-  private static final Log LOG = LogFactory.getLog(OptionsParser.class);
+  static final Log LOG = LogFactory.getLog(OptionsParser.class);
 
   private static final Options cliOptions = new Options();
 
@@ -88,14 +88,26 @@ public class OptionsParser {
 
     DistCpOptions option = parseSourceAndTargetPaths(command);
 
-    //Process all the other option switches and set options appropriately
-    if (command.hasOption(DistCpOptionSwitch.IGNORE_FAILURES.getSwitch())) {
-      option.setIgnoreFailures(true);
-    }
+    option.setIgnoreFailures(
+        command.hasOption(DistCpOptionSwitch.IGNORE_FAILURES.getSwitch()));
 
-    if (command.hasOption(DistCpOptionSwitch.ATOMIC_COMMIT.getSwitch())) {
-      option.setAtomicCommit(true);
-    }
+    option.setAtomicCommit(
+        command.hasOption(DistCpOptionSwitch.ATOMIC_COMMIT.getSwitch()));
+
+    option.setSyncFolder(
+        command.hasOption(DistCpOptionSwitch.SYNC_FOLDERS.getSwitch()));
+
+    option.setOverwrite(
+        command.hasOption(DistCpOptionSwitch.OVERWRITE.getSwitch()));
+
+    option.setAppend(
+        command.hasOption(DistCpOptionSwitch.APPEND.getSwitch()));
+
+    option.setDeleteMissing(
+        command.hasOption(DistCpOptionSwitch.DELETE_MISSING.getSwitch()));
+
+    option.setSkipCRC(
+        command.hasOption(DistCpOptionSwitch.SKIP_CRC.getSwitch()));
 
     if (command.hasOption(DistCpOptionSwitch.WORK_PATH.getSwitch()) &&
         option.shouldAtomicCommit()) {
@@ -111,25 +123,6 @@ public class OptionsParser {
       option.setLogPath(new Path(getVal(command, DistCpOptionSwitch.LOG_PATH.getSwitch())));
     }
 
-    if (command.hasOption(DistCpOptionSwitch.SYNC_FOLDERS.getSwitch())) {
-      option.setSyncFolder(true);
-    }
-
-    if (command.hasOption(DistCpOptionSwitch.OVERWRITE.getSwitch())) {
-      option.setOverwrite(true);
-    }
-
-    if (command.hasOption(DistCpOptionSwitch.APPEND.getSwitch())) {
-      option.setAppend(true);
-    }
-
-    if (command.hasOption(DistCpOptionSwitch.DELETE_MISSING.getSwitch())) {
-      option.setDeleteMissing(true);
-    }
-
-    if (command.hasOption(DistCpOptionSwitch.SKIP_CRC.getSwitch())) {
-      option.setSkipCRC(true);
-    }
 
     if (command.hasOption(DistCpOptionSwitch.BLOCKING.getSwitch())) {
       option.setBlocking(false);
@@ -164,6 +157,8 @@ public class OptionsParser {
           DistCpOptionSwitch.FILTERS.getSwitch()));
     }
 
+    option.validate();
+
     return option;
   }
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/03788d30/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
index b0626ba..35109cc 100644
--- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
+++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.tools;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 import org.junit.Assert;
@@ -691,11 +692,25 @@ public class TestOptionsParser {
     }
 
     try {
-      OptionsParser.parse(new String[] { "-diff", "s1", "s2", "-update", "-delete",
+      options = OptionsParser.parse(new String[] {
+          "-diff", "s1", "s2", "-update", "-delete",
           "hdfs://localhost:9820/source/first",
           "hdfs://localhost:9820/target/" });
-      fail("-diff should fail if -delete option is specified");
+      assertFalse("-delete should be ignored when -diff is specified",
+          options.shouldDeleteMissing());
     } catch (IllegalArgumentException e) {
+      fail("Got unexpected IllegalArgumentException: " + e.getMessage());
+    }
+
+    try {
+      options = OptionsParser.parse(new String[] {
+          "-diff", "s1", "s2", "-delete",
+          "hdfs://localhost:9820/source/first",
+          "hdfs://localhost:9820/target/" });
+      fail("-diff should fail if -update option is not specified");
+    } catch (IllegalArgumentException e) {
+      assertFalse("-delete should be ignored when -diff is specified",
+          options.shouldDeleteMissing());
       GenericTestUtils.assertExceptionContains(
           "Diff is valid only with update options", e);
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


Mime
View raw message