You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ae...@apache.org on 2016/05/23 23:20:47 UTC
[09/50] hadoop git commit: HDFS-10397. Distcp should ignore -delete
option if -diff option is provided instead of exiting. Contributed by
Mingliang Liu.
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/HDFS-1312
Commit: 03788d3015c962eac1a35fa5df39356e8b84731c
Parents: 9478484
Author: Jing Zhao <ji...@apache.org>
Authored: Tue May 17 15:44:07 2016 -0700
Committer: Jing Zhao <ji...@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