You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-issues@hadoop.apache.org by "Bhallamudi Venkata Siva Kamesh (JIRA)" <ji...@apache.org> on 2011/01/05 12:29:46 UTC

[jira] Created: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Close all the file streams propely in a finally block to avoid their leakage.
-----------------------------------------------------------------------------

                 Key: MAPREDUCE-2243
                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
             Project: Hadoop Map/Reduce
          Issue Type: Improvement
          Components: jobtracker, tasktracker
    Affects Versions: 0.20.1, 0.22.0
         Environment: NA
            Reporter: Bhallamudi Venkata Siva Kamesh
            Priority: Minor
             Fix For: 0.22.0


In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.

CompletedJobStatusStore.java
------------------------------------------
       dataOut.writeInt(events.length);
        for (TaskCompletionEvent event : events) {
          event.write(dataOut);
        }
       dataOut.close() ;

EventWriter.java
----------------------
   encoder.flush();
   out.close();

MapTask.java
-------------------
    splitMetaInfo.write(out);
     out.close();

TaskLog
------------
 1) str = fis.readLine();
      fis.close();

2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
      .toString()).length() - prevLogLength) + "\n");
    dos.close();

TotalOrderPartitioner.java
-----------------------------------
 while (reader.next(key, value)) {
	      parts.add(key);
	      key = ReflectionUtils.newInstance(keyClass, conf);
	    }
reader.close();




-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072836#comment-13072836 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

I have verified the tests manually by running the "ant test". 

These changes are related to streams closure, verified manually, new tests are not needed for this.


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072338#comment-13072338 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

Updated the patch as per the above comments.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072745#comment-13072745 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

Please find the "ant test-patch" result from my system.
{code:xml} 

   [exec] 
     [exec] -1 overall.  
     [exec] 
     [exec]     +1 @author.  The patch does not contain any @author tags.
     [exec] 
     [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
     [exec]                         Please justify why no new tests are needed for this patch.
     [exec]                         Also please list what manual steps were performed to verify this patch.
     [exec] 
     [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
     [exec] 
     [exec]     +1 javac. The applied patch does not increase the total number of javac compiler warnings.
     [exec] 
     [exec]     +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
     [exec] 
     [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
     [exec] 
     [exec]     +1 system test framework. The patch passed system test framework compile.
     [exec] 
     [exec] 
{code} 



> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072188#comment-13072188 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

Yes Nicholas, Good catch. I missed it. 
I will update the patch with single try, that will handle the above cases also.


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Affects Version/s:     (was: 0.20.1)

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Attachment: MAPREDUCE-2243-1.patch

Thanks Todd for reviewing. Update the patch with review comments fix.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Attachment: MAPREDUCE-2243-3.patch

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13057955#comment-13057955 ] 

Eli Collins commented on MAPREDUCE-2243:
----------------------------------------

fyi HADOOP-7428 is a case where the RTE is relevant.  

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13066557#comment-13066557 ] 

Hadoop QA commented on MAPREDUCE-2243:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12486739/MAPREDUCE-2243-2.patch
  against trunk revision 1146517.

    +1 @author.  The patch does not contain any @author tags.

    -1 tests included.  The patch doesn't appear to include any new or modified tests.
                        Please justify why no new tests are needed for this patch.
                        Also please list what manual steps were performed to verify this patch.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    -1 findbugs.  The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    -1 core tests.  The patch failed these core unit tests:
                  org.apache.hadoop.cli.TestMRCLI
                  org.apache.hadoop.fs.TestFileSystem

    -1 contrib tests.  The patch failed contrib unit tests.

    +1 system test framework.  The patch passed system test framework compile.

Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/474//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/474//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/474//console

This message is automatically generated.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13061998#comment-13061998 ] 

Hadoop QA commented on MAPREDUCE-2243:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12485733/MAPREDUCE-2243-1.patch
  against trunk revision 1144097.

    +1 @author.  The patch does not contain any @author tags.

    -1 tests included.  The patch doesn't appear to include any new or modified tests.
                        Please justify why no new tests are needed for this patch.
                        Also please list what manual steps were performed to verify this patch.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs (version 1.3.9) warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    -1 core tests.  The patch failed these core unit tests:
                  org.apache.hadoop.cli.TestMRCLI
                  org.apache.hadoop.fs.TestFileSystem
                  org.apache.hadoop.mapred.TestIsolationRunner
                  org.apache.hadoop.mapred.TestMiniMRWithDFS
                  org.apache.hadoop.mapred.TestSeveral
                  org.apache.hadoop.security.authorize.TestServiceLevelAuthorization

    -1 contrib tests.  The patch failed contrib unit tests.

    +1 system test framework.  The patch passed system test framework compile.

Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/447//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/447//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/447//console

This message is automatically generated.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082071#comment-13082071 ] 

Hudson commented on MAPREDUCE-2243:
-----------------------------------

Integrated in Hadoop-Mapreduce-trunk #751 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk/751/])
    MAPREDUCE-2243. Close streams propely in a finally-block to avoid leakage in CompletedJobStatusStore, TaskLog, EventWriter and TotalOrderPartitioner.  Contributed by Devaraj K

szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1152787
Files : 
* /hadoop/common/trunk/mapreduce/CHANGES.txt
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapreduce/lib/partition/TotalOrderPartitioner.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapred/TaskLog.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapred/CompletedJobStatusStore.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapreduce/jobhistory/EventWriter.java


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243-4.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Status: Open  (was: Patch Available)

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082057#comment-13082057 ] 

Hudson commented on MAPREDUCE-2243:
-----------------------------------

Integrated in Hadoop-Mapreduce-trunk-Commit #760 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/760/])
    MAPREDUCE-2243. Close streams propely in a finally-block to avoid leakage in CompletedJobStatusStore, TaskLog, EventWriter and TotalOrderPartitioner.  Contributed by Devaraj K

szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1152787
Files : 
* /hadoop/common/trunk/mapreduce/CHANGES.txt
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapreduce/lib/partition/TotalOrderPartitioner.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapred/TaskLog.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapred/CompletedJobStatusStore.java
* /hadoop/common/trunk/mapreduce/src/java/org/apache/hadoop/mapreduce/jobhistory/EventWriter.java


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243-4.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Owen O'Malley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12979644#action_12979644 ] 

Owen O'Malley commented on MAPREDUCE-2243:
------------------------------------------

In most cases, the exceptions outside of IOException don't matter much because they will bring down 
the service. In cases where the system will stay up, I'd suggest using:

{code}
try {
  f = fs.open(...);
  f.write(...);
  f.close(...);
} catch (IOException ie) {
  IOUtils.cleanup(LOG, f);
  throw ie;
} catch (RuntimeException re) {
  IOUtils.cleanup(LOG, f);
  throw re;
} catch (Error err) {
  IOUtils.cleanup(LOG, f);
  throw err;
}
{code}

this leaves the nominal case simple. Note that this is the worst case, if we get an Error every system in Hadoop should shutdown.
There is no point in continuing and worrying about lost file handles at that point is too extreme. Also note that with Java's
garbage collector, this is far less critical, even for a server, than in C.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>             Fix For: 0.22.0
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072611#comment-13072611 ] 

Tsz Wo (Nicholas), SZE commented on MAPREDUCE-2243:
---------------------------------------------------

All hudson machines are down. Could you run "ant test" and "ant test-patch" manually?

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated MAPREDUCE-2243:
----------------------------------------------

    Resolution: Fixed
        Status: Resolved  (was: Patch Available)

+1 patch looks good

I have committed this. Thanks, Devaraj!

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243-4.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Updated: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Attachment: MAPREDUCE-2243.patch

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Laxman (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12982466#action_12982466 ] 

Laxman commented on MAPREDUCE-2243:
-----------------------------------

@Owen

bq. In most cases, the exceptions outside of IOException don't matter much because they will bring down.
bq. this leaves the nominal case simple. Note that this is the worst case, if we get an Error every system in Hadoop should shutdown.
bq. There is no point in continuing and worrying about lost file handles at that point is too extreme. 

Yes, I agree to your point in *Error* scenarios. How about some runtime exception which need not be handled in the positive flow?

Handling unexpected generic exceptions and errors will result in catch and rethrow pattern. So, I prefer to handle the stream closure in try block as well as in finally block.

As per your initial comments Kamesh has corrected to close the streams in try block as well as in finally block.
Do you still see some issue with this approach? 
How handling stream close in catch block is better than handling in try and finally blocks? 

My opinion on this issue is "Handling stream closures in try and finally block is fool proof and it will avoid some code duplication."

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated MAPREDUCE-2243:
----------------------------------------------

    Hadoop Flags: [Reviewed]

+1 patch looks good.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Fix Version/s: 0.23.0

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Owen O'Malley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12977849#action_12977849 ] 

Owen O'Malley commented on MAPREDUCE-2243:
------------------------------------------

Finally blocks have some very bad properties for exceptions. In particular, they tend to mask errors.

{code:title=bad example}
try {
  f = fs.open(...);
  f.write(...);
} finally {
  f.close();
}
{code}

because if an exception is thrown in the close it will mask exceptions thrown in main body. 

To make the problem concrete, if you don't have permission to write the file, it will throw an IOException saying so. But then the finally block will get a NullPointerException on the close and the user will get that exception without the original exception.

The preferred style is to do:

{code:title=good example}
try {
  f = fs.open(...);
  f.write(...);
  f.close();
} catch (IOException ioe) {
  IOUtils.cleanup(LOG, f);
  throw ioe;
}
{code}

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>             Fix For: 0.22.0
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Attachment: MAPREDUCE-2243-2.patch

Updated the patch based on the trunk latest changes.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Assigned] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K reassigned MAPREDUCE-2243:
------------------------------------

    Assignee: Devaraj K

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072164#comment-13072164 ] 

Tsz Wo (Nicholas), SZE commented on MAPREDUCE-2243:
---------------------------------------------------

In TaskLog.getLogFileDetail(..), should we use a single try-finally for everything?


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13073508#comment-13073508 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

I fixed and updated the patch. Thanks Nicholas.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243-4.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Updated: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Affects Version/s: 0.23.0
               Status: Patch Available  (was: Open)

Provided patch for trunk as per the above comments.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072183#comment-13072183 ] 

Tsz Wo (Nicholas), SZE commented on MAPREDUCE-2243:
---------------------------------------------------

Hi Devaraj, is it the case the if str == null, then fis won't be closed?  Similarly, if {{str.indexOf(LogFileDetail.LOCATION)+LogFileDetail.LOCATION.length()}} is out of range, then substring(..) will throw exception and fis won't be closed.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Status: Patch Available  (was: Open)

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj K updated MAPREDUCE-2243:
---------------------------------

    Attachment: MAPREDUCE-2243-4.patch

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243-4.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13073486#comment-13073486 ] 

Tsz Wo (Nicholas), SZE commented on MAPREDUCE-2243:
---------------------------------------------------

Hi Devaraj, I was trying to commit the patch but there was an additional {{encoder.flush()}} in {{EventWriter.close()}}.  Could you fix it?

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243-3.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13066789#comment-13066789 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

The below lines of code is showing as the cause for the findbug issue which is already present and not added by the patch.

{code:title=TaskLog.java|borderStyle=solid}
    if (localFS == null) {// set localFS once
      localFS = FileSystem.getLocal(new Configuration());
    }
{code}

These changes are related to streams closure, verified manually, test cases are not needed for this.

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Bhallamudi Venkata Siva Kamesh (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12979580#action_12979580 ] 

Bhallamudi Venkata Siva Kamesh commented on MAPREDUCE-2243:
-----------------------------------------------------------

Hi Owen, 
The above example given is great and describes when to close the streams clearly. But there is chance of missing the streams closure as given below. 

If try block throws any exception other than IOException, then the chances of stream not getting closed will be there.

{code:title=Given style of closing streams|borderStyle=solid}
try {
  f = fs.open(...);
  f.write(...); 
  // If any exception other than IOException thrown from here will lead to missing the stream close
  f.close();
} catch (IOException ioe) {
  IOUtils.cleanup(LOG, f);
  throw ioe;
}
{code}

So consider the following approach. 
{code:title=Proposed style of closing streams|borderStyle=solid}
	try	{
		f = fs.open(....);
		f.write(....);
		f.close();
		f = null; // set to null explicitly so that close in finally will not execute again and also just to avoid double close issue, if any.
	}
	catch(IOException ioe) {
		throw ioe;
	}
	finally {
		IOUtils.cleanup(LOG, f); // This will do nothing if the try block completed successfully. 
	}
{code} 

But I feel we need to check the feasibility of the getting other kinds of exception in try block except the ones which we are catching!. 
If all kinds of exceptions have been handled then I feel no need of final block.



> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.20.1, 0.22.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Priority: Minor
>             Fix For: 0.22.0
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13061686#comment-13061686 ] 

Todd Lipcon commented on MAPREDUCE-2243:
----------------------------------------

this is useless code, shows up 5x:
+      } catch (IOException ioe) {
+        throw ioe;

otherwise seems reasonable

> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>         Attachments: MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (MAPREDUCE-2243) Close all the file streams propely in a finally block to avoid their leakage.

Posted by "Devaraj K (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MAPREDUCE-2243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13072176#comment-13072176 ] 

Devaraj K commented on MAPREDUCE-2243:
--------------------------------------

Thanks Nicholas for reviewing.
It is not required to use single try for everything because the block of code (given below) which comes in between these two try blocks is not required to keep in the try block.
{code:xml}
if (str == null) { //the file doesn't have anything
      throw new IOException ("Index file for the log of " + taskid+" doesn't exist.");
    }
    l.location = str.substring(str.indexOf(LogFileDetail.LOCATION)+
        LogFileDetail.LOCATION.length());
    //special cases are the debugout and profile.out files. They are guaranteed
    //to be associated with each task attempt since jvm reuse is disabled
    //when profiling/debugging is enabled
    if (filter.equals(LogName.DEBUGOUT) || filter.equals(LogName.PROFILE)) {
      l.length = new File(l.location, filter.toString()).length();
      l.start = 0;
      fis.close();
      return l;
    }
{code}

Here it has only fis.close(); which deals with streams. If we put fis.close() in the try finally, it will try to close it in the try block, if suppose it throws IOException, then again it will try to close in the finally and throws the exception again. It doesn’t give any difference if we keep in try or don’t keep inside try block.


> Close all the file streams propely in a finally block to avoid their leakage.
> -----------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-2243
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2243
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker, tasktracker
>    Affects Versions: 0.22.0, 0.23.0
>         Environment: NA
>            Reporter: Bhallamudi Venkata Siva Kamesh
>            Assignee: Devaraj K
>            Priority: Minor
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-2243-1.patch, MAPREDUCE-2243-2.patch, MAPREDUCE-2243.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> In the following classes streams should be closed in finally block to avoid their leakage in the exceptional cases.
> CompletedJobStatusStore.java
> ------------------------------------------
>        dataOut.writeInt(events.length);
>         for (TaskCompletionEvent event : events) {
>           event.write(dataOut);
>         }
>        dataOut.close() ;
> EventWriter.java
> ----------------------
>    encoder.flush();
>    out.close();
> MapTask.java
> -------------------
>     splitMetaInfo.write(out);
>      out.close();
> TaskLog
> ------------
>  1) str = fis.readLine();
>       fis.close();
> 2) dos.writeBytes(Long.toString(new File(logLocation, LogName.SYSLOG
>       .toString()).length() - prevLogLength) + "\n");
>     dos.close();
> TotalOrderPartitioner.java
> -----------------------------------
>  while (reader.next(key, value)) {
> 	      parts.add(key);
> 	      key = ReflectionUtils.newInstance(keyClass, conf);
> 	    }
> reader.close();

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira