[ 
https://issues.apache.org/jira/browse/HADOOP-11283?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14258685#comment-14258685
 ] 

Tsuyoshi OZAWA commented on HADOOP-11283:
-----------------------------------------

[~varun_saxena] Overall latest patch looks good to me. Minor nits:

1. These lines exceed 80 LOC. Could you fix them?
{code}
+    try (SequenceFile.Writer src_writer = SequenceFile.createWriter(jobfs, 
jobConf,
+            srcfilelist, LongWritable.class, FilePair.class, 
SequenceFile.CompressionType.NONE);
+         SequenceFile.Writer dst_writer = SequenceFile.createWriter(jobfs, 
jobConf,
+            dstfilelist, Text.class, Text.class, 
SequenceFile.CompressionType.NONE);
+         SequenceFile.Writer dir_writer = SequenceFile.createWriter(jobfs, 
jobConf,
+            dstdirlist, Text.class, FilePair.class, 
SequenceFile.CompressionType.NONE);) {
{code}

I prefer to fix these lines like this:
{code}
    try (
        SequenceFile.Writer src_writer = SequenceFile.createWriter(jobfs,
            jobConf, srcfilelist, LongWritable.class, FilePair.class,
            SequenceFile.CompressionType.NONE);
        SequenceFile.Writer dst_writer = SequenceFile.createWriter(jobfs,
            jobConf, dstfilelist, Text.class, Text.class,
            SequenceFile.CompressionType.NONE);
        SequenceFile.Writer dir_writer = SequenceFile.createWriter(jobfs,
            jobConf, dstdirlist, Text.class, FilePair.class,
            SequenceFile.CompressionType.NONE)
    ) {
{code}

2. A following line includes trailing space.
{code}
+      HashSet<Path> parentDirsToCopy = new HashSet<Path>(); 
{code}

Once these comments are addressed, I'll commit it.

> Potentially unclosed SequenceFile.Writer in DistCpV1#setup()
> ------------------------------------------------------------
>
>                 Key: HADOOP-11283
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11283
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: Varun Saxena
>            Priority: Minor
>         Attachments: HADOOP-11283.001.patch, HADOOP-11283.patch
>
>
> {code}
>     SequenceFile.Writer src_writer = SequenceFile.createWriter(jobfs, jobConf,
>         srcfilelist, LongWritable.class, FilePair.class,
>         SequenceFile.CompressionType.NONE);
>     Path dstfilelist = new Path(jobDirectory, "_distcp_dst_files");
>     SequenceFile.Writer dst_writer = SequenceFile.createWriter(jobfs, jobConf,
>         dstfilelist, Text.class, Text.class,
>         SequenceFile.CompressionType.NONE);
> {code}
> If creation of dst_writer throws exception, src_writer would be left unclosed 
> since there is no finally clause doing that for the above code.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to