From 856bc4e28b356f31d3c48eed2c352fb9010232d7 Mon Sep 17 00:00:00 2001 From: Jing Zhao Date: Wed, 22 Jun 2016 15:15:47 -0700 Subject: [PATCH] HDFS-10556. DistCpOptions should be validated automatically. Contributed by Mingliang Liu. --- .../apache/hadoop/tools/DistCpOptions.java | 45 +- .../apache/hadoop/tools/OptionsParser.java | 47 +- .../hadoop/tools/TestDistCpOptions.java | 510 ++++++++++++++++++ .../hadoop/tools/TestOptionsParser.java | 13 - 4 files changed, 572 insertions(+), 43 deletions(-) create mode 100644 hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.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 2a962a61442..b81bc038af2 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 @@ -163,6 +163,7 @@ public class DistCpOptions { * @param atomicCommit - boolean switch */ public void setAtomicCommit(boolean atomicCommit) { + validate(DistCpOptionSwitch.ATOMIC_COMMIT, atomicCommit); this.atomicCommit = atomicCommit; } @@ -181,6 +182,7 @@ public class DistCpOptions { * @param syncFolder - boolean switch */ public void setSyncFolder(boolean syncFolder) { + validate(DistCpOptionSwitch.SYNC_FOLDERS, syncFolder); this.syncFolder = syncFolder; } @@ -199,7 +201,22 @@ public class DistCpOptions { * @param deleteMissing - boolean switch */ public void setDeleteMissing(boolean deleteMissing) { + validate(DistCpOptionSwitch.DELETE_MISSING, deleteMissing); this.deleteMissing = deleteMissing; + ignoreDeleteMissingIfUseDiff(); + } + + /** + * -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. + */ + private void ignoreDeleteMissingIfUseDiff() { + if (deleteMissing && useDiff) { + OptionsParser.LOG.warn("-delete and -diff are mutually exclusive. " + + "The -delete option will be ignored."); + deleteMissing = false; + } } /** @@ -254,6 +271,7 @@ public class DistCpOptions { * @param overwrite - boolean switch */ public void setOverwrite(boolean overwrite) { + validate(DistCpOptionSwitch.OVERWRITE, overwrite); this.overwrite = overwrite; } @@ -269,6 +287,7 @@ public class DistCpOptions { * update option and CRC is not skipped. */ public void setAppend(boolean append) { + validate(DistCpOptionSwitch.APPEND, append); this.append = append; } @@ -285,9 +304,11 @@ 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; + ignoreDeleteMissingIfUseDiff(); } public void disableUsingDiff() { @@ -311,6 +332,7 @@ public class DistCpOptions { * @param skipCRC - boolean switch */ public void setSkipCRC(boolean skipCRC) { + validate(DistCpOptionSwitch.SKIP_CRC, skipCRC); this.skipCRC = skipCRC; } @@ -565,15 +587,20 @@ public class DistCpOptions { this.filtersFile = filtersFilename; } - 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); - } + 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); if (syncFolder && atomicCommit) { throw new IllegalArgumentException("Atomic commit can't be used with " + 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 414068c4d5a..b3a40efa4e8 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 @@ -88,26 +88,14 @@ public class OptionsParser { DistCpOptions option = parseSourceAndTargetPaths(command); - option.setIgnoreFailures( - command.hasOption(DistCpOptionSwitch.IGNORE_FAILURES.getSwitch())); + //Process all the other option switches and set options appropriately + if (command.hasOption(DistCpOptionSwitch.IGNORE_FAILURES.getSwitch())) { + option.setIgnoreFailures(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.ATOMIC_COMMIT.getSwitch())) { + option.setAtomicCommit(true); + } if (command.hasOption(DistCpOptionSwitch.WORK_PATH.getSwitch()) && option.shouldAtomicCommit()) { @@ -123,6 +111,25 @@ 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); @@ -162,8 +169,6 @@ public class OptionsParser { DistCpOptionSwitch.FILTERS.getSwitch())); } - option.validate(); - return option; } diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java new file mode 100644 index 00000000000..5a56cc31ee5 --- /dev/null +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java @@ -0,0 +1,510 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.tools; + +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.tools.DistCpOptions.FileAttribute; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Collections; + +import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; +import static org.apache.hadoop.tools.DistCpOptions.maxNumListstatusThreads; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; + +/** + * This is to test constructing {@link DistCpOptions} manually with setters. + * + * The test cases in this class is very similar to the parser test, see + * {@link TestOptionsParser}. + */ +public class TestDistCpOptions { + + @Test + public void testSetIgnoreFailure() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldIgnoreFailures()); + + options.setIgnoreFailures(true); + Assert.assertTrue(options.shouldIgnoreFailures()); + } + + @Test + public void testSetOverwrite() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldOverwrite()); + + options.setOverwrite(true); + Assert.assertTrue(options.shouldOverwrite()); + + try { + options.setSyncFolder(true); + Assert.fail("Update and overwrite aren't allowed together"); + } catch (IllegalArgumentException ignore) { + } + } + + @Test + public void testLogPath() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertNull(options.getLogPath()); + + final Path logPath = new Path("hdfs://localhost:8020/logs"); + options.setLogPath(logPath); + Assert.assertEquals(logPath, options.getLogPath()); + } + + @Test + public void testSetBlokcing() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertTrue(options.shouldBlock()); + + options.setBlocking(false); + Assert.assertFalse(options.shouldBlock()); + } + + @Test + public void testSetBandwidth() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertEquals(DistCpConstants.DEFAULT_BANDWIDTH_MB, + options.getMapBandwidth()); + + options.setMapBandwidth(11); + Assert.assertEquals(11, options.getMapBandwidth()); + } + + @Test(expected = AssertionError.class) + public void testSetNonPositiveBandwidth() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setMapBandwidth(-11); + } + + @Test(expected = AssertionError.class) + public void testSetZeroBandwidth() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setMapBandwidth(0); + } + + @Test + public void testSetSkipCRC() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldSkipCRC()); + + options.setSyncFolder(true); + options.setSkipCRC(true); + Assert.assertTrue(options.shouldSyncFolder()); + Assert.assertTrue(options.shouldSkipCRC()); + } + + @Test + public void testSetAtomicCommit() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldAtomicCommit()); + + options.setAtomicCommit(true); + Assert.assertTrue(options.shouldAtomicCommit()); + + try { + options.setSyncFolder(true); + Assert.fail("Atomic and sync folders were mutually exclusive"); + } catch (IllegalArgumentException ignore) { + } + } + + @Test + public void testSetWorkPath() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertNull(options.getAtomicWorkPath()); + + options.setAtomicCommit(true); + Assert.assertNull(options.getAtomicWorkPath()); + + final Path workPath = new Path("hdfs://localhost:8020/work"); + options.setAtomicWorkPath(workPath); + Assert.assertEquals(workPath, + options.getAtomicWorkPath()); + } + + @Test + public void testSetSyncFolders() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldSyncFolder()); + + options.setSyncFolder(true); + Assert.assertTrue(options.shouldSyncFolder()); + } + + @Test + public void testSetDeleteMissing() { + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldDeleteMissing()); + + options.setSyncFolder(true); + options.setDeleteMissing(true); + Assert.assertTrue(options.shouldSyncFolder()); + Assert.assertTrue(options.shouldDeleteMissing()); + } + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setOverwrite(true); + options.setDeleteMissing(true); + Assert.assertTrue(options.shouldOverwrite()); + Assert.assertTrue(options.shouldDeleteMissing()); + } + try { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setDeleteMissing(true); + fail("Delete missing should fail without update or overwrite options"); + } catch (IllegalArgumentException e) { + assertExceptionContains("Delete missing is applicable only with update " + + "or overwrite options", e); + } + try { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setDeleteMissing(true); + options.setUseDiff(true, "s1", "s2"); + assertFalse("-delete should be ignored when -diff is specified", + options.shouldDeleteMissing()); + } catch (IllegalArgumentException e) { + fail("Got unexpected IllegalArgumentException: " + e.getMessage()); + } + } + + @Test + public void testSetSSLConf() { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertNull(options.getSslConfigurationFile()); + + options.setSslConfigurationFile("/tmp/ssl-client.xml"); + Assert.assertEquals("/tmp/ssl-client.xml", + options.getSslConfigurationFile()); + } + + @Test + public void testSetMaps() { + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertEquals(DistCpConstants.DEFAULT_MAPS, options.getMaxMaps()); + + options.setMaxMaps(1); + Assert.assertEquals(1, options.getMaxMaps()); + } + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setMaxMaps(0); + Assert.assertEquals(1, options.getMaxMaps()); + } + } + + @Test + public void testSetNumListtatusThreads() { + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + // If command line argument isn't set, we expect .getNumListstatusThreads + // option to be zero (so that we know when to override conf properties). + Assert.assertEquals(0, options.getNumListstatusThreads()); + + options.setNumListstatusThreads(12); + Assert.assertEquals(12, options.getNumListstatusThreads()); + } + + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setNumListstatusThreads(0); + Assert.assertEquals(0, options.getNumListstatusThreads()); + } + + // Ignore large number of threads. + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + options.setNumListstatusThreads(maxNumListstatusThreads * 2); + Assert.assertEquals(maxNumListstatusThreads, + options.getNumListstatusThreads()); + } + } + + @Test + public void testSourceListing() { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + Assert.assertEquals(new Path("hdfs://localhost:8020/source/first"), + options.getSourceFileListing()); + } + + @Test(expected = AssertionError.class) + public void testMissingTarget() { + new DistCpOptions(new Path("hdfs://localhost:8020/source/first"), null); + } + + @Test + public void testToString() { + DistCpOptions option = new DistCpOptions(new Path("abc"), new Path("xyz")); + final String val = "DistCpOptions{atomicCommit=false, syncFolder=false, " + + "deleteMissing=false, ignoreFailures=false, overwrite=false, " + + "skipCRC=false, blocking=true, numListstatusThreads=0, maxMaps=20, " + + "mapBandwidth=100, sslConfigurationFile='null', " + + "copyStrategy='uniformsize', preserveStatus=[], " + + "preserveRawXattrs=false, atomicWorkPath=null, logPath=null, " + + "sourceFileListing=abc, sourcePaths=null, targetPath=xyz, " + + "targetPathExists=true, filtersFile='null'}"; + String optionString = option.toString(); + Assert.assertEquals(val, optionString); + Assert.assertNotSame(DistCpOptionSwitch.ATOMIC_COMMIT.toString(), + DistCpOptionSwitch.ATOMIC_COMMIT.name()); + } + + @Test + public void testCopyStrategy() { + { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + Assert.assertEquals(DistCpConstants.UNIFORMSIZE, + options.getCopyStrategy()); + options.setCopyStrategy("dynamic"); + Assert.assertEquals("dynamic", options.getCopyStrategy()); + } + } + + @Test + public void testTargetPath() { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + Assert.assertEquals(new Path("hdfs://localhost:8020/target/"), + options.getTargetPath()); + } + + @Test + public void testPreserve() { + { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(options.shouldPreserve(FileAttribute.BLOCKSIZE)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.REPLICATION)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.PERMISSION)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.USER)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.GROUP)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.CHECKSUMTYPE)); + } + { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.preserve(FileAttribute.ACL); + Assert.assertFalse(options.shouldPreserve(FileAttribute.BLOCKSIZE)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.REPLICATION)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.PERMISSION)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.USER)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.GROUP)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.CHECKSUMTYPE)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.ACL)); + } + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source/")), + new Path("hdfs://localhost:8020/target/")); + options.preserve(FileAttribute.BLOCKSIZE); + options.preserve(FileAttribute.REPLICATION); + options.preserve(FileAttribute.PERMISSION); + options.preserve(FileAttribute.USER); + options.preserve(FileAttribute.GROUP); + options.preserve(FileAttribute.CHECKSUMTYPE); + + Assert.assertTrue(options.shouldPreserve(FileAttribute.BLOCKSIZE)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.REPLICATION)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.PERMISSION)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.USER)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.GROUP)); + Assert.assertTrue(options.shouldPreserve(FileAttribute.CHECKSUMTYPE)); + Assert.assertFalse(options.shouldPreserve(FileAttribute.XATTR)); + } + } + + @Test + public void testAppendOption() { + { + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source/")), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setAppend(true); + Assert.assertTrue(options.shouldAppend()); + } + + try { + // make sure -append is only valid when -update is specified + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source/")), + new Path("hdfs://localhost:8020/target/")); + options.setAppend(true); + fail("Append should fail if update option is not specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains( + "Append is valid only with update options", e); + } + + try { + // make sure -append is invalid when skipCrc is specified + final DistCpOptions options = new DistCpOptions( + Collections.singletonList(new Path("hdfs://localhost:8020/source/")), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setAppend(true); + options.setSkipCRC(true); + fail("Append should fail if skipCrc option is specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains( + "Append is disallowed when skipping CRC", e); + } + } + + @Test + public void testDiffOption() { + { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setUseDiff(true, "s1", "s2"); + Assert.assertTrue(options.shouldUseDiff()); + Assert.assertEquals("s1", options.getFromSnapshot()); + Assert.assertEquals("s2", options.getToSnapshot()); + } + { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setUseDiff(true, "s1", "."); + Assert.assertTrue(options.shouldUseDiff()); + Assert.assertEquals("s1", options.getFromSnapshot()); + Assert.assertEquals(".", options.getToSnapshot()); + } + + // make sure -diff is only valid when -update is specified + try { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setUseDiff(true, "s1", "s2"); + fail("-diff should fail if -update option is not specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains( + "Diff is valid only with update options", e); + } + + try { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setSyncFolder(true); + options.setUseDiff(true, "s1", "s2"); + options.setDeleteMissing(true); + assertFalse("-delete should be ignored when -diff is specified", + options.shouldDeleteMissing()); + } catch (IllegalArgumentException e) { + fail("Got unexpected IllegalArgumentException: " + e.getMessage()); + } + + try { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setUseDiff(true, "s1", "s2"); + options.setDeleteMissing(true); + fail("-diff should fail if -update option is not specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains( + "Diff is valid only with update options", e); + } + + try { + final DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + options.setDeleteMissing(true); + options.setUseDiff(true, "s1", "s2"); + fail("-delete should fail if -update option is not specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains("Delete missing is applicable only with update " + + "or overwrite options", e); + } + } + + @Test + public void testExclusionsOption() { + DistCpOptions options = new DistCpOptions( + new Path("hdfs://localhost:8020/source/first"), + new Path("hdfs://localhost:8020/target/")); + Assert.assertNull(options.getFiltersFile()); + + options.setFiltersFile("/tmp/filters.txt"); + Assert.assertEquals("/tmp/filters.txt", options.getFiltersFile()); + } +} 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 946722176cd..4dd05de40b1 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 @@ -715,19 +715,6 @@ public class TestOptionsParser { 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); - } - try { OptionsParser.parse(new String[] { "-diff", "s1", "s2", "-delete", "-overwrite",