[
https://issues.apache.org/jira/browse/HDFS-7312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14208914#comment-14208914
]
Yongjun Zhang commented on HDFS-7312:
-------------------------------------
Hi [~jprosser],
Thanks for the nice work here. I looked through, it looks good, except for a
couple of improvements and nits:
Improvements:
* For the following new test created, to achieve better code sharing
{code}
/** copy files from dfs file system to dfs file system with skiptmp */
public void testCopyFromDfsToDfsWithSkiptmp() throws Exception {
{code}
suggest to create a new private method with an additional boolean parameter
skipTmp, {{private void testCopyFromDfsToDfs(final boolean skipTmp}}. Then you
can make the pre-existing test testCopyFromDfsToDfs and your new test to call
this private method with false and true accordingly.
* This same above suggestion applies to {{testCopySingleFileWithSkiptmp()}}.
* Line 1221, do we really need to ceate the tmpDir here? For example, if we
copy a single file to a destination file, why a tmpDir is needed? I think we
can avoid doing this (removing that block of code), and have a file-existence
checking when doing the fullyDelete. The point is, the TMP_DIR_LABEL may not
always be created, we don't have to create it just for the purpose of deleting
it. Right?
Nits:
* About usage description "Copy files directly to the final destination.",
maybe we can change it to "Instead, copy files directly to the final
destination." ?
* Line 394, "// filename and the path becomes its parent directory.", replace
"path" with "destPath"
* Line 1218, rename "tmpDirRoot" to "tmpDirPrefix"?
* Line 396, tab key not replaced with spaces.
* line 400 remove newly added extra newline
* Line 956, replace "The job to configure" with "The job configuration"
* Line 1218, line is too long, need to be <= 80 columns based on hadoop code
guideline;
BTW, I guess you have tried to test it out in real clusters, right?
Thanks a lot.
> Update DistCp v1 to optionally not use tmp location
> ---------------------------------------------------
>
> Key: HDFS-7312
> URL: https://issues.apache.org/jira/browse/HDFS-7312
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: tools
> Affects Versions: 2.5.1
> Reporter: Joseph Prosser
> Assignee: Joseph Prosser
> Priority: Minor
> Attachments: HDFS-7312.001.patch, HDFS-7312.002.patch,
> HDFS-7312.003.patch, HDFS-7312.patch
>
> Original Estimate: 72h
> Remaining Estimate: 72h
>
> DistCp v1 currently copies files to a tmp location and then renames that to
> the specified destination. This can cause performance issues on filesystems
> such as S3. A -skiptmp flag will be added to bypass this step and copy
> directly to the destination. This feature mirrors a similar one added to
> HBase ExportSnapshot
> [HBASE-11119|https://issues.apache.org/jira/browse/HBASE-11119]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
