You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by abellina <gi...@git.apache.org> on 2016/07/26 05:23:54 UTC

[GitHub] storm pull request #1593: STORM-1995: close input stream and other tweaks to...

GitHub user abellina opened a pull request:

    https://github.com/apache/storm/pull/1593

    STORM-1995: close input stream and other tweaks to downloadChunk

    When the input stream is being removed from its parent collection (after reading fully) in downloadChunk it isn't being closed, causing a leak.
    
    Thanks to @tgravescs for making this change.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/abellina/storm STORM-1995_downloadChunk_should_close_input_stream

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/1593.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1593
    
----
commit f38982c022e38dd7e970babfdfaf941064ebfba2
Author: Alessandro Bellina <ab...@yahoo-inc.com>
Date:   2016-07-21T05:26:20Z

    STORM-1995: close input stream and other tweaks to downloadChunk

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1593: STORM-1995: close input stream and other tweaks to...

Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1593#discussion_r72246238
  
    --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
    @@ -1793,16 +1793,16 @@
             (.mark nimbus:num-downloadChunk-calls)
             (check-authorization! nimbus nil nil "fileDownload")
             (let [downloaders (:downloaders nimbus)
    -              ^BufferFileInputStream is (.get downloaders id)]
    +              ^BufferInputStream is (.get downloaders id)]
    --- End diff --
    
    @HeartSaVioR, in beginFileDownload (just above the impl for downloadChunk) the type used for the input stream and put into :downloaders is a BufferInputStream, hence the change in downloadChunk.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1593: STORM-1995: close input stream and other tweaks to...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/1593


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1593: STORM-1995: close input stream and other tweaks to...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1593#discussion_r72244414
  
    --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
    @@ -1793,16 +1793,16 @@
             (.mark nimbus:num-downloadChunk-calls)
             (check-authorization! nimbus nil nil "fileDownload")
             (let [downloaders (:downloaders nimbus)
    -              ^BufferFileInputStream is (.get downloaders id)]
    +              ^BufferInputStream is (.get downloaders id)]
    --- End diff --
    
    Is there a reason to change type hint class name?
    http://storm.apache.org/releases/1.0.0/javadocs/org/apache/storm/utils/BufferFileInputStream.html
    BufferFileInputStream is not a "is-a" BufferInputStream, and it also has close() method itself.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1593: STORM-1995: close input stream and other tweaks to downlo...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/1593
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1593: STORM-1995: close input stream and other tweaks to...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1593#discussion_r72246867
  
    --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj ---
    @@ -1793,16 +1793,16 @@
             (.mark nimbus:num-downloadChunk-calls)
             (check-authorization! nimbus nil nil "fileDownload")
             (let [downloaders (:downloaders nimbus)
    -              ^BufferFileInputStream is (.get downloaders id)]
    +              ^BufferInputStream is (.get downloaders id)]
    --- End diff --
    
    @abellina OK I didn't know about the context. Great change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---