From fd951e3a37d60a545742c74a4006dabceb72e431 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Tue, 5 Sep 2017 21:45:30 -0700 Subject: [PATCH] HADOOP-14839. DistCp log output should contain copied and deleted files and directories. Contributed by Yiqun Lin. (cherry picked from commit 7ba61235dba87bc292134194044d54d9a12a4d7c) --- .../apache/hadoop/tools/DistCpConstants.java | 1 + .../hadoop/tools/DistCpOptionSwitch.java | 7 ++ .../apache/hadoop/tools/DistCpOptions.java | 22 +++++ .../apache/hadoop/tools/OptionsParser.java | 4 +- .../hadoop/tools/mapred/CopyMapper.java | 32 ++++-- .../src/site/markdown/DistCp.md.vm | 1 + .../hadoop/tools/TestDistCpOptions.java | 21 +++- .../hadoop/tools/mapred/TestCopyMapper.java | 99 ++++++++++++++++++- .../hadoop/tools/DistCp_Counter.properties | 1 + 9 files changed, 177 insertions(+), 11 deletions(-) diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java index 8991e0933db..0bae5d50345 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java @@ -45,6 +45,7 @@ public class DistCpConstants { public static final String CONF_LABEL_ATOMIC_COPY = "distcp.atomic.copy"; public static final String CONF_LABEL_WORK_PATH = "distcp.work.path"; public static final String CONF_LABEL_LOG_PATH = "distcp.log.path"; + public static final String CONF_LABEL_VERBOSE_LOG = "distcp.verbose.log"; public static final String CONF_LABEL_IGNORE_FAILURES = "distcp.ignore.failures"; public static final String CONF_LABEL_PRESERVE_STATUS = "distcp.preserve.status"; public static final String CONF_LABEL_PRESERVE_RAWXATTRS = diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java index 016172e02f6..e1c824e1c35 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java @@ -118,6 +118,13 @@ public enum DistCpOptionSwitch { LOG_PATH(DistCpConstants.CONF_LABEL_LOG_PATH, new Option("log", true, "Folder on DFS where distcp execution logs are saved")), + /** + * Log additional info (path, size) in the SKIP/COPY log. + */ + VERBOSE_LOG(DistCpConstants.CONF_LABEL_VERBOSE_LOG, + new Option("v", false, + "Log additional info (path, size) in the SKIP/COPY log")), + /** * Copy strategy is use. This could be dynamic or uniform size etc. * DistCp would use an appropriate input format based on this. 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 af6cb8be03a..ece1a9426ef 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 @@ -101,6 +101,9 @@ public final class DistCpOptions { // content at their s1, if src is not the same as tgt. private final boolean useRdiff; + /** Whether to log additional info (path, size) in the SKIP/COPY log. */ + private final boolean verboseLog; + // For both -diff and -rdiff, given the example command line switches, two // steps are taken: // 1. Sync Step. This step does renaming/deletion ops in the snapshot diff, @@ -204,6 +207,7 @@ public final class DistCpOptions { this.blocksPerChunk = builder.blocksPerChunk; this.copyBufferSize = builder.copyBufferSize; + this.verboseLog = builder.verboseLog; } public Path getSourceFileListing() { @@ -323,6 +327,10 @@ public final class DistCpOptions { return copyBufferSize; } + public boolean shouldVerboseLog() { + return verboseLog; + } + /** * Add options to configuration. These will be used in the Mapper/committer * @@ -361,6 +369,8 @@ public final class DistCpOptions { String.valueOf(blocksPerChunk)); DistCpOptionSwitch.addToConf(conf, DistCpOptionSwitch.COPY_BUFFER_SIZE, String.valueOf(copyBufferSize)); + DistCpOptionSwitch.addToConf(conf, DistCpOptionSwitch.VERBOSE_LOG, + String.valueOf(verboseLog)); } /** @@ -396,6 +406,7 @@ public final class DistCpOptions { ", filtersFile='" + filtersFile + '\'' + ", blocksPerChunk=" + blocksPerChunk + ", copyBufferSize=" + copyBufferSize + + ", verboseLog=" + verboseLog + '}'; } @@ -420,6 +431,7 @@ public final class DistCpOptions { private boolean append = false; private boolean skipCRC = false; private boolean blocking = true; + private boolean verboseLog = false; private boolean useDiff = false; private boolean useRdiff = false; @@ -552,6 +564,11 @@ public final class DistCpOptions { throw new IllegalArgumentException( "-diff and -rdiff are mutually exclusive"); } + + if (verboseLog && logPath == null) { + throw new IllegalArgumentException( + "-v is valid only with -log option"); + } } @VisibleForTesting @@ -685,6 +702,11 @@ public final class DistCpOptions { : DistCpConstants.COPY_BUFFER_SIZE_DEFAULT; return this; } + + public Builder withVerboseLog(boolean newVerboseLog) { + this.verboseLog = newVerboseLog; + return this; + } } } 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 2bfaccfe096..606ed3254f4 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 @@ -111,7 +111,9 @@ public class OptionsParser { .withCRC( command.hasOption(DistCpOptionSwitch.SKIP_CRC.getSwitch())) .withBlocking( - !command.hasOption(DistCpOptionSwitch.BLOCKING.getSwitch())); + !command.hasOption(DistCpOptionSwitch.BLOCKING.getSwitch())) + .withVerboseLog( + command.hasOption(DistCpOptionSwitch.VERBOSE_LOG.getSwitch())); if (command.hasOption(DistCpOptionSwitch.DIFF.getSwitch())) { String[] snapshots = getVals(command, diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyMapper.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyMapper.java index d6b3ba817b0..faa4aa275a3 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyMapper.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyMapper.java @@ -54,6 +54,7 @@ public class CopyMapper extends Mapper */ public static enum Counter { COPY, // Number of files received by the mapper for copy. + DIR_COPY, // Number of directories received by the mapper for copy. SKIP, // Number of files skipped. FAIL, // Number of files that failed to be copied. BYTESCOPIED, // Number of bytes actually copied by the copy-mapper, total. @@ -82,6 +83,7 @@ public class CopyMapper extends Mapper private boolean skipCrc = false; private boolean overWrite = false; private boolean append = false; + private boolean verboseLog = false; private EnumSet preserve = EnumSet.noneOf(FileAttribute.class); private FileSystem targetFS = null; @@ -105,6 +107,8 @@ public class CopyMapper extends Mapper skipCrc = conf.getBoolean(DistCpOptionSwitch.SKIP_CRC.getConfigLabel(), false); overWrite = conf.getBoolean(DistCpOptionSwitch.OVERWRITE.getConfigLabel(), false); append = conf.getBoolean(DistCpOptionSwitch.APPEND.getConfigLabel(), false); + verboseLog = conf.getBoolean( + DistCpOptionSwitch.VERBOSE_LOG.getConfigLabel(), false); preserve = DistCpUtils.unpackAttributes(conf.get(DistCpOptionSwitch. PRESERVE_STATUS.getConfigLabel())); @@ -196,6 +200,13 @@ public class CopyMapper extends Mapper updateSkipCounters(context, sourceCurrStatus); context.write(null, new Text("SKIP: " + sourceCurrStatus.getPath())); + if (verboseLog) { + context.write(null, + new Text("FILE_SKIPPED: source=" + sourceFileStatus.getPath() + + ", size=" + sourceFileStatus.getLen() + " --> " + + "target=" + target + ", size=" + (targetStatus == null ? + 0 : targetStatus.getLen()))); + } } else { if (sourceCurrStatus.isSplit()) { tmpTarget = DistCpUtils.getSplitChunkPath(target, sourceCurrStatus); @@ -203,8 +214,8 @@ public class CopyMapper extends Mapper if (LOG.isDebugEnabled()) { LOG.debug("copying " + sourceCurrStatus + " " + tmpTarget); } - copyFileWithRetry(description, sourceCurrStatus, tmpTarget, context, - action, fileAttributes); + copyFileWithRetry(description, sourceCurrStatus, tmpTarget, + targetStatus, context, action, fileAttributes); } DistCpUtils.preserve(target.getFileSystem(conf), tmpTarget, sourceCurrStatus, fileAttributes, preserveRawXattrs); @@ -235,9 +246,10 @@ public class CopyMapper extends Mapper } private void copyFileWithRetry(String description, - CopyListingFileStatus sourceFileStatus, Path target, Context context, - FileAction action, EnumSet fileAttributes) - throws IOException { + CopyListingFileStatus sourceFileStatus, Path target, + FileStatus targrtFileStatus, Context context, FileAction action, + EnumSet fileAttributes) + throws IOException, InterruptedException { long bytesCopied; try { bytesCopied = (Long) new RetriableFileCopyCommand(skipCrc, description, @@ -251,6 +263,14 @@ public class CopyMapper extends Mapper incrementCounter(context, Counter.BYTESCOPIED, bytesCopied); incrementCounter(context, Counter.COPY, 1); totalBytesCopied += bytesCopied; + + if (verboseLog) { + context.write(null, + new Text("FILE_COPIED: source=" + sourceFileStatus.getPath() + "," + + " size=" + sourceFileStatus.getLen() + " --> " + + "target=" + target + ", size=" + (targrtFileStatus == null ? + 0 : targrtFileStatus.getLen()))); + } } private void createTargetDirsWithRetry(String description, @@ -260,7 +280,7 @@ public class CopyMapper extends Mapper } catch (Exception e) { throw new IOException("mkdir failed for " + target, e); } - incrementCounter(context, Counter.COPY, 1); + incrementCounter(context, Counter.DIR_COPY, 1); } private static void updateSkipCounters(Context context, diff --git a/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm b/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm index 925b24ef111..2cd01e21aeb 100644 --- a/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm +++ b/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm @@ -220,6 +220,7 @@ Flag | Description | Notes `-p[rbugpcaxt]` | Preserve r: replication number b: block size u: user g: group p: permission c: checksum-type a: ACL x: XAttr t: timestamp | When `-update` is specified, status updates will **not** be synchronized unless the file sizes also differ (i.e. unless the file is re-created). If -pa is specified, DistCp preserves the permissions also because ACLs are a super-set of permissions. The option -pr is only valid if both source and target directory are not erasure coded. **Note:** If -p option's are not specified, then by default block size is preserved. `-i` | Ignore failures | As explained in the Appendix, this option will keep more accurate statistics about the copy than the default case. It also preserves logs from failed copies, which can be valuable for debugging. Finally, a failing map will not cause the job to fail before all splits are attempted. `-log ` | Write logs to \ | DistCp keeps logs of each file it attempts to copy as map output. If a map fails, the log output will not be retained if it is re-executed. +`-v` | Log additional info (path, size) in the SKIP/COPY log | This option can only be used with -log option. `-m ` | Maximum number of simultaneous copies | Specify the number of maps to copy data. Note that more maps may not necessarily improve throughput. `-overwrite` | Overwrite destination | If a map fails and `-i` is not specified, all the files in the split, not only those that failed, will be recopied. As discussed in the Usage documentation, it also changes the semantics for generating destination paths, so users should use this carefully. `-update` | Overwrite if source and destination differ in size, blocksize, or checksum | As noted in the preceding, this is not a "sync" operation. The criteria examined are the source and destination file sizes, blocksizes, and checksums; if they differ, the source file replaces the destination file. As discussed in the Usage documentation, it also changes the semantics for generating destination paths, so users should use this carefully. 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 index 6b59b974401..dd8ec697b65 100644 --- 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 @@ -287,7 +287,7 @@ public class TestDistCpOptions { "mapBandwidth=0.0, copyStrategy='uniformsize', preserveStatus=[], " + "atomicWorkPath=null, logPath=null, sourceFileListing=abc, " + "sourcePaths=null, targetPath=xyz, filtersFile='null'," + - " blocksPerChunk=0, copyBufferSize=8192}"; + " blocksPerChunk=0, copyBufferSize=8192, verboseLog=false}"; String optionString = option.toString(); Assert.assertEquals(val, optionString); Assert.assertNotSame(DistCpOptionSwitch.ATOMIC_COMMIT.toString(), @@ -514,4 +514,23 @@ public class TestDistCpOptions { Assert.assertEquals(DistCpConstants.COPY_BUFFER_SIZE_DEFAULT, builder.build().getCopyBufferSize()); } + + @Test + public void testVerboseLog() { + final DistCpOptions.Builder builder = new DistCpOptions.Builder( + Collections.singletonList(new Path("hdfs://localhost:8020/source")), + new Path("hdfs://localhost:8020/target/")); + Assert.assertFalse(builder.build().shouldVerboseLog()); + + try { + builder.withVerboseLog(true).build(); + fail("-v should fail if -log option is not specified"); + } catch (IllegalArgumentException e) { + assertExceptionContains("-v is valid only with -log option", e); + } + + final Path logPath = new Path("hdfs://localhost:8020/logs"); + builder.withLogPath(logPath).withVerboseLog(true); + Assert.assertTrue(builder.build().shouldVerboseLog()); + } } diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyMapper.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyMapper.java index 866ad6e178e..fd998c82c66 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyMapper.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyMapper.java @@ -255,7 +255,13 @@ public class TestCopyMapper { context.getConfiguration().setBoolean( DistCpOptionSwitch.APPEND.getConfigLabel(), true); copyMapper.setup(context); + + int numFiles = 0; for (Path path: pathList) { + if (fs.getFileStatus(path).isFile()) { + numFiles++; + } + copyMapper.map(new Text(DistCpUtils.getRelativePath(new Path(SOURCE_PATH), path)), new CopyListingFileStatus(cluster.getFileSystem().getFileStatus( path)), context); @@ -266,7 +272,7 @@ public class TestCopyMapper { Assert.assertEquals(nFiles * DEFAULT_FILE_SIZE * 2, stubContext .getReporter().getCounter(CopyMapper.Counter.BYTESCOPIED) .getValue()); - Assert.assertEquals(pathList.size(), stubContext.getReporter(). + Assert.assertEquals(numFiles, stubContext.getReporter(). getCounter(CopyMapper.Counter.COPY).getValue()); } @@ -295,7 +301,15 @@ public class TestCopyMapper { copyMapper.setup(context); - for (Path path: pathList) { + int numFiles = 0; + int numDirs = 0; + for (Path path : pathList) { + if (fs.getFileStatus(path).isDirectory()) { + numDirs++; + } else { + numFiles++; + } + copyMapper.map( new Text(DistCpUtils.getRelativePath(new Path(SOURCE_PATH), path)), new CopyListingFileStatus(fs.getFileStatus(path)), context); @@ -303,8 +317,10 @@ public class TestCopyMapper { // Check that the maps worked. verifyCopy(fs, preserveChecksum); - Assert.assertEquals(pathList.size(), stubContext.getReporter() + Assert.assertEquals(numFiles, stubContext.getReporter() .getCounter(CopyMapper.Counter.COPY).getValue()); + Assert.assertEquals(numDirs, stubContext.getReporter() + .getCounter(CopyMapper.Counter.DIR_COPY).getValue()); if (!preserveChecksum) { Assert.assertEquals(nFiles * DEFAULT_FILE_SIZE, stubContext .getReporter().getCounter(CopyMapper.Counter.BYTESCOPIED) @@ -1118,4 +1134,81 @@ public class TestCopyMapper { e.printStackTrace(); } } + + @Test + public void testVerboseLogging() throws Exception { + deleteState(); + createSourceData(); + + FileSystem fs = cluster.getFileSystem(); + CopyMapper copyMapper = new CopyMapper(); + StubContext stubContext = new StubContext(getConfiguration(), null, 0); + Mapper.Context context + = stubContext.getContext(); + copyMapper.setup(context); + + int numFiles = 0; + for (Path path : pathList) { + if (fs.getFileStatus(path).isFile()) { + numFiles++; + } + + copyMapper.map( + new Text(DistCpUtils.getRelativePath(new Path(SOURCE_PATH), path)), + new CopyListingFileStatus(fs.getFileStatus(path)), context); + } + + // Check that the maps worked. + Assert.assertEquals(numFiles, stubContext.getReporter() + .getCounter(CopyMapper.Counter.COPY).getValue()); + + testCopyingExistingFiles(fs, copyMapper, context); + // verify the verbose log + // we shouldn't print verbose log since this option is disabled + for (Text value : stubContext.getWriter().values()) { + Assert.assertTrue(!value.toString().startsWith("FILE_COPIED:")); + Assert.assertTrue(!value.toString().startsWith("FILE_SKIPPED:")); + } + + // test with verbose logging + deleteState(); + createSourceData(); + + stubContext = new StubContext(getConfiguration(), null, 0); + context = stubContext.getContext(); + copyMapper.setup(context); + + // enables verbose logging + context.getConfiguration().setBoolean( + DistCpOptionSwitch.VERBOSE_LOG.getConfigLabel(), true); + copyMapper.setup(context); + + for (Path path : pathList) { + copyMapper.map( + new Text(DistCpUtils.getRelativePath(new Path(SOURCE_PATH), path)), + new CopyListingFileStatus(fs.getFileStatus(path)), context); + } + + Assert.assertEquals(numFiles, stubContext.getReporter() + .getCounter(CopyMapper.Counter.COPY).getValue()); + + // verify the verbose log of COPY log + int numFileCopied = 0; + for (Text value : stubContext.getWriter().values()) { + if (value.toString().startsWith("FILE_COPIED:")) { + numFileCopied++; + } + } + Assert.assertEquals(numFiles, numFileCopied); + + // verify the verbose log of SKIP log + int numFileSkipped = 0; + testCopyingExistingFiles(fs, copyMapper, context); + for (Text value : stubContext.getWriter().values()) { + if (value.toString().startsWith("FILE_SKIPPED:")) { + numFileSkipped++; + } + } + Assert.assertEquals(numFiles, numFileSkipped); + } } diff --git a/hadoop-tools/hadoop-extras/src/main/java/org/apache/hadoop/tools/DistCp_Counter.properties b/hadoop-tools/hadoop-extras/src/main/java/org/apache/hadoop/tools/DistCp_Counter.properties index 2234619715a..77f6c02e6bd 100644 --- a/hadoop-tools/hadoop-extras/src/main/java/org/apache/hadoop/tools/DistCp_Counter.properties +++ b/hadoop-tools/hadoop-extras/src/main/java/org/apache/hadoop/tools/DistCp_Counter.properties @@ -15,6 +15,7 @@ CounterGroupName= distcp COPY.name= Files copied +DIR_COPY.name= Directories copied SKIP.name= Files skipped FAIL.name= Files failed BYTESCOPIED.name= Bytes copied