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

(cherry picked from commit 03788d3015)
This commit is contained in:
Jing Zhao 2016-05-17 15:44:07 -07:00
parent 937bc924cb
commit dd7f5a3b88
3 changed files with 52 additions and 54 deletions

View File

@ -163,7 +163,6 @@ public class DistCpOptions {
* @param atomicCommit - boolean switch
*/
public void setAtomicCommit(boolean atomicCommit) {
validate(DistCpOptionSwitch.ATOMIC_COMMIT, atomicCommit);
this.atomicCommit = atomicCommit;
}
@ -182,7 +181,6 @@ public class DistCpOptions {
* @param syncFolder - boolean switch
*/
public void setSyncFolder(boolean syncFolder) {
validate(DistCpOptionSwitch.SYNC_FOLDERS, syncFolder);
this.syncFolder = syncFolder;
}
@ -201,7 +199,6 @@ public class DistCpOptions {
* @param deleteMissing - boolean switch
*/
public void setDeleteMissing(boolean deleteMissing) {
validate(DistCpOptionSwitch.DELETE_MISSING, deleteMissing);
this.deleteMissing = deleteMissing;
}
@ -257,7 +254,6 @@ public class DistCpOptions {
* @param overwrite - boolean switch
*/
public void setOverwrite(boolean overwrite) {
validate(DistCpOptionSwitch.OVERWRITE, overwrite);
this.overwrite = overwrite;
}
@ -273,7 +269,6 @@ public class DistCpOptions {
* update option and CRC is not skipped.
*/
public void setAppend(boolean append) {
validate(DistCpOptionSwitch.APPEND, append);
this.append = append;
}
@ -290,7 +285,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;
@ -317,7 +311,6 @@ public class DistCpOptions {
* @param skipCRC - boolean switch
*/
public void setSkipCRC(boolean skipCRC) {
validate(DistCpOptionSwitch.SKIP_CRC, skipCRC);
this.skipCRC = skipCRC;
}
@ -572,20 +565,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 " +
@ -614,7 +602,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");
}

View File

@ -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);
@ -169,6 +162,8 @@ public class OptionsParser {
DistCpOptionSwitch.FILTERS.getSwitch()));
}
option.validate();
return option;
}

View File

@ -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;
@ -400,7 +401,7 @@ public class TestOptionsParser {
String val = "DistCpOptions{atomicCommit=false, syncFolder=false, "
+ "deleteMissing=false, ignoreFailures=false, overwrite=false, "
+ "skipCRC=false, blocking=true, numListstatusThreads=0, maxMaps=20, "
+ "mapBandwidth=100.0, sslConfigurationFile='null', "
+ "mapBandwidth=100, sslConfigurationFile='null', "
+ "copyStrategy='uniformsize', preserveStatus=[], "
+ "preserveRawXattrs=false, atomicWorkPath=null, logPath=null, "
+ "sourceFileListing=abc, sourcePaths=null, targetPath=xyz, "
@ -704,11 +705,25 @@ public class TestOptionsParser {
}
try {
OptionsParser.parse(new String[] { "-diff", "s1", "s2", "-update", "-delete",
"hdfs://localhost:8020/source/first",
"hdfs://localhost:8020/target/" });
fail("-diff should fail if -delete option is specified");
options = OptionsParser.parse(new String[] {
"-diff", "s1", "s2", "-update", "-delete",
"hdfs://localhost:9820/source/first",
"hdfs://localhost:9820/target/" });
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);
}