[
https://issues.apache.org/jira/browse/HADOOP-16158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16816703#comment-16816703
]
Steve Loughran commented on HADOOP-16158:
-----------------------------------------
Production code looks good -no concerns there.
TestCopyCommitter.java
* can you add the new imports in the org.apache bit of the import section.
Normally the org.apache.* block should have an empty line between it and the
others, but I see this test already breaks that -just add the new imports
inside the hadoop ones so it doesn't get any more confused.
* L420: should just catch IOException
* L423: rethrow the exception instead of raising a failure but losing the root
cause.
* L435: Nice bit of work in {{createSrcAndWorkFilesWithDifferentChecksum}}. Can
you add some javadocs explaining how it does this for our successors to
understand. It took me a moment to work out, and I know (vaguely) how checksums
work.
Now that {{compareFileLengthsAndChecksums}} is isolated to a static method, can
you add test which checks what it does in isolation, again with
{{createSrcAndWorkFilesWithDifferentChecksum}}. This will isolate regressions.
FYI For backporting, distcp <= Hadoop 3.1- uses commons-logging, so that log
statement will also need to change for those versions, even before you worry
about branch-2
> DistCp to supports checksum validation when copy blocks in parallel
> -------------------------------------------------------------------
>
> Key: HADOOP-16158
> URL: https://issues.apache.org/jira/browse/HADOOP-16158
> Project: Hadoop Common
> Issue Type: Improvement
> Components: tools/distcp
> Affects Versions: 3.2.0, 2.9.2
> Reporter: Kai Xie
> Assignee: Kai Xie
> Priority: Major
> Attachments: HADOOP-16158-001.patch, HADOOP-16158-002.patch,
> HADOOP-16158-003.patch, HADOOP-16158-004.patch
>
>
> Copying blocks in parallel (enabled when blocks per chunk > 0) is a great
> DistCp improvement that can hugely speed up copying big files.
> But its checksum validation is skipped, e.g. in
> `RetriableFileCopyCommand.java`
>
> {code:java}
> if (!source.isSplit()) {
> compareCheckSums(sourceFS, source.getPath(), sourceChecksum,
> targetFS, targetPath);
> }
> {code}
> and this could result in checksum/data mismatch without notifying
> developers/users (e.g. HADOOP-16049).
> I'd like to provide a patch to add the checksum validation.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
