From 8d09a623cb057619e9ec5d66fe13a0107dff1ec4 Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 27 Apr 2023 19:10:55 +0800 Subject: [PATCH 1/6] HADOOP-18723. Add detail logs if distcp checksum mismatch --- .../main/java/org/apache/hadoop/tools/util/DistCpUtils.java | 2 ++ .../org/apache/hadoop/tools/mapred/TestCopyCommitter.java | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java index e77b2031a76db..3daf558eb947b 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java @@ -592,10 +592,12 @@ public static CopyMapper.ChecksumComparison checksumsAreEqual( // comparison that took place and return not compatible. // else if matched, return compatible with the matched result. if (sourceChecksum == null || targetChecksum == null) { + LOG.error("Checksum incompatible. Source checksum: {}, target checksum: {}", sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.INCOMPATIBLE; } else if (sourceChecksum.equals(targetChecksum)) { return CopyMapper.ChecksumComparison.TRUE; } + LOG.error("Checksum not equal. Source checksum: {}, target checksum: {}", sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.FALSE; } diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java index 6a537dc6e7de9..0cd54a023e18d 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java @@ -19,6 +19,8 @@ package org.apache.hadoop.tools.mapred; import org.apache.hadoop.fs.contract.ContractTestUtils; +import org.apache.hadoop.hdfs.server.balancer.NameNodeConnector; +import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; @@ -513,7 +515,8 @@ public void testCommitWithChecksumMismatchWithoutSkipCrc() private void testCommitWithChecksumMismatch(boolean skipCrc) throws IOException { - + GenericTestUtils.LogCapturer log = GenericTestUtils.LogCapturer.captureLogs( + LoggerFactory.getLogger(DistCpUtils.class)); TaskAttemptContext taskAttemptContext = getTaskAttemptContext(config); JobContext jobContext = new JobContextImpl( taskAttemptContext.getConfiguration(), @@ -569,6 +572,7 @@ private void testCommitWithChecksumMismatch(boolean skipCrc) fs, new Path(sourceBase + srcFilename), null, fs, new Path(targetBase + srcFilename), sourceCurrStatus.getLen())); + assertTrue(log.getOutput().contains("Checksum not equal")); } catch(IOException exception) { if (skipCrc) { LOG.error("Unexpected exception is found", exception); From b1060a84e3874aa8cee2d5db6e6fa493187c9753 Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 27 Apr 2023 19:15:24 +0800 Subject: [PATCH 2/6] HADOOP-18723. Remove unused imports --- .../java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java index 0cd54a023e18d..fa31b7b0b2472 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java @@ -19,8 +19,6 @@ package org.apache.hadoop.tools.mapred; import org.apache.hadoop.fs.contract.ContractTestUtils; -import org.apache.hadoop.hdfs.server.balancer.NameNodeConnector; -import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; From a832a9b6341c3764f7f8f5e47ab3a67628dd8357 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 28 Apr 2023 06:55:22 +0800 Subject: [PATCH 3/6] HADOOP-18723. Fix checkstyle --- .../main/java/org/apache/hadoop/tools/util/DistCpUtils.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java index 3daf558eb947b..2b9fe33bedaef 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java @@ -592,12 +592,14 @@ public static CopyMapper.ChecksumComparison checksumsAreEqual( // comparison that took place and return not compatible. // else if matched, return compatible with the matched result. if (sourceChecksum == null || targetChecksum == null) { - LOG.error("Checksum incompatible. Source checksum: {}, target checksum: {}", sourceChecksum, targetChecksum); + LOG.error("Checksum incompatible. Source checksum: {}, target checksum: {}", + sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.INCOMPATIBLE; } else if (sourceChecksum.equals(targetChecksum)) { return CopyMapper.ChecksumComparison.TRUE; } - LOG.error("Checksum not equal. Source checksum: {}, target checksum: {}", sourceChecksum, targetChecksum); + LOG.error("Checksum not equal. Source checksum: {}, target checksum: {}", + sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.FALSE; } From 462b34034693ace140c5dcfcd16bb682fde6c58c Mon Sep 17 00:00:00 2001 From: Symious Date: Wed, 3 May 2023 14:56:21 +0800 Subject: [PATCH 4/6] HADOOP-18723. Add log only for mismatch --- .../main/java/org/apache/hadoop/tools/util/DistCpUtils.java | 4 +--- .../org/apache/hadoop/tools/mapred/TestCopyCommitter.java | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java index 2b9fe33bedaef..fdc7a2de5f2e9 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java @@ -592,13 +592,11 @@ public static CopyMapper.ChecksumComparison checksumsAreEqual( // comparison that took place and return not compatible. // else if matched, return compatible with the matched result. if (sourceChecksum == null || targetChecksum == null) { - LOG.error("Checksum incompatible. Source checksum: {}, target checksum: {}", - sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.INCOMPATIBLE; } else if (sourceChecksum.equals(targetChecksum)) { return CopyMapper.ChecksumComparison.TRUE; } - LOG.error("Checksum not equal. Source checksum: {}, target checksum: {}", + LOG.info("Checksum not equal. Source checksum: {}, target checksum: {}", sourceChecksum, targetChecksum); return CopyMapper.ChecksumComparison.FALSE; } diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java index fa31b7b0b2472..8690e7319a06e 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java @@ -57,6 +57,7 @@ import static org.apache.hadoop.tools.DistCpConstants.CONF_LABEL_TARGET_FINAL_PATH; import static org.apache.hadoop.tools.DistCpConstants.CONF_LABEL_TARGET_WORK_PATH; import static org.apache.hadoop.tools.util.TestDistCpUtils.*; +import static org.hamcrest.CoreMatchers.containsString; public class TestCopyCommitter { private static final Logger LOG = LoggerFactory.getLogger(TestCopyCommitter.class); @@ -570,7 +571,7 @@ private void testCommitWithChecksumMismatch(boolean skipCrc) fs, new Path(sourceBase + srcFilename), null, fs, new Path(targetBase + srcFilename), sourceCurrStatus.getLen())); - assertTrue(log.getOutput().contains("Checksum not equal")); + assertThat(log.getOutput(), containsString("Checksum not equal")); } catch(IOException exception) { if (skipCrc) { LOG.error("Unexpected exception is found", exception); From c99bcea56a3e78c3d98e3bcc77a398350a1d8976 Mon Sep 17 00:00:00 2001 From: Symious Date: Wed, 3 May 2023 17:01:21 +0800 Subject: [PATCH 5/6] HADOOP-18723. Replace deprecated function --- .../java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java index 8690e7319a06e..5813c6cac7bd4 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java @@ -58,6 +58,7 @@ import static org.apache.hadoop.tools.DistCpConstants.CONF_LABEL_TARGET_WORK_PATH; import static org.apache.hadoop.tools.util.TestDistCpUtils.*; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; public class TestCopyCommitter { private static final Logger LOG = LoggerFactory.getLogger(TestCopyCommitter.class); From c73e1da394e70693d14372c139dc0b2f8f38475b Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 5 May 2023 10:01:31 +0800 Subject: [PATCH 6/6] HADOOP-18723. Add source and target path --- .../main/java/org/apache/hadoop/tools/util/DistCpUtils.java | 5 +++-- .../org/apache/hadoop/tools/mapred/TestCopyCommitter.java | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java index fdc7a2de5f2e9..7ba06eac3a46c 100644 --- a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java +++ b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/util/DistCpUtils.java @@ -596,8 +596,9 @@ public static CopyMapper.ChecksumComparison checksumsAreEqual( } else if (sourceChecksum.equals(targetChecksum)) { return CopyMapper.ChecksumComparison.TRUE; } - LOG.info("Checksum not equal. Source checksum: {}, target checksum: {}", - sourceChecksum, targetChecksum); + LOG.info("Checksum not equal. Source path: {}, source checksum: {}, target path {}, " + + "target checksum: {}", + source, sourceChecksum, target, targetChecksum); return CopyMapper.ChecksumComparison.FALSE; } diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java index 5813c6cac7bd4..6fac7aedb6ea5 100644 --- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java +++ b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java @@ -19,6 +19,7 @@ package org.apache.hadoop.tools.mapred; import org.apache.hadoop.fs.contract.ContractTestUtils; +import org.assertj.core.api.Assertions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; @@ -57,8 +58,6 @@ import static org.apache.hadoop.tools.DistCpConstants.CONF_LABEL_TARGET_FINAL_PATH; import static org.apache.hadoop.tools.DistCpConstants.CONF_LABEL_TARGET_WORK_PATH; import static org.apache.hadoop.tools.util.TestDistCpUtils.*; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.MatcherAssert.assertThat; public class TestCopyCommitter { private static final Logger LOG = LoggerFactory.getLogger(TestCopyCommitter.class); @@ -572,7 +571,7 @@ private void testCommitWithChecksumMismatch(boolean skipCrc) fs, new Path(sourceBase + srcFilename), null, fs, new Path(targetBase + srcFilename), sourceCurrStatus.getLen())); - assertThat(log.getOutput(), containsString("Checksum not equal")); + Assertions.assertThat(log.getOutput()).contains("Checksum not equal"); } catch(IOException exception) { if (skipCrc) { LOG.error("Unexpected exception is found", exception);