You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by nssalian <gi...@git.apache.org> on 2016/07/26 22:53:02 UTC
[GitHub] flink pull request #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileC...
GitHub user nssalian opened a pull request:
https://github.com/apache/flink/pull/2299
[FLINK-4259]: Unclosed FSDataOutputStream in FileCache#copy(): Added a \u2026
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful description of your changes.
Description of Changes:
Add a close statement for closing the FSDataOutputStream.
Requesting review.
- [ ] General
- The pull request references the related JIRA issue ("[FLINK-4259] Unclosed FSDataOutputStream in FileCache#copy()")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
\u2026close statement to close the FSDataOutputStream object
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nssalian/flink FLINK-4259
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2299.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 #2299
----
commit f77780aa9c997eeac3950cfd2194b7c08851ef8d
Author: Neelesh Srinivas Salian <ns...@cloudera.com>
Date: 2016-07-25T20:30:42Z
FLINK-4259: Unclosed FSDataOutputStream in FileCache#copy(): Added a close statement to close the FSDataOutputStream object
----
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by nssalian <gi...@git.apache.org>.
Github user nssalian commented on the issue:
https://github.com/apache/flink/pull/2299
Hi @mbalassi and @zentol, I ran the builds twice and both instances the PythonBinderTest passed.
The 2nd build ran with a failure on 2 with:
MVN exited with EXIT CODE: 1.
java.io.FileNotFoundException: build-target/lib/flink-dist-*.jar (No such file or directory)
which, if I am not wrong, isn't related here.
Please let me know if you need me to add anything additional.
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by mbalassi <gi...@git.apache.org>.
Github user mbalassi commented on the issue:
https://github.com/apache/flink/pull/2299
The code change is very concise and looks right, the one issue that bugs me is that there are no direct tests in place to verify it - and it is not necessarily straight forward to test as it is a resource leak issue.
I tried building the project locally and in fact `PythonPlanBinderTest` that uses the change transitively got stuck, but I could not manage to reproduce the issue when running solely that test from IntelliJ.
I have fired up a travis test myself and the PR is also triggering one, let us whether those go through.
@nssalian do you see a straight forward way to test the behaviour?
PS: Could you enabled the travis build on your repo, please? (As Flink comes with a travis.yml all you need to do is to go to Travis, login with your github user and hit enable on the repo.)
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by nssalian <gi...@git.apache.org>.
Github user nssalian commented on the issue:
https://github.com/apache/flink/pull/2299
hi @mbalassim thank you for the review. I have kicked off a build for my flink repo. Let me observe what happens.
With regards to a test, I was thinking something on the lines of FileCacheDeleteValidationTest.java with creating and checking whether the output stream is closed.
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by nssalian <gi...@git.apache.org>.
Github user nssalian commented on the issue:
https://github.com/apache/flink/pull/2299
Thanks @mbalassi
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by mbalassi <gi...@git.apache.org>.
Github user mbalassi commented on the issue:
https://github.com/apache/flink/pull/2299
@nssalian has just reached out to me in person. I will have a look at this.
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by nssalian <gi...@git.apache.org>.
Github user nssalian commented on the issue:
https://github.com/apache/flink/pull/2299
Hi @zentol , thank you.I ran the build once locally and it went through. Just kicked off another for sanity.
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by nssalian <gi...@git.apache.org>.
Github user nssalian commented on the issue:
https://github.com/apache/flink/pull/2299
Thanks @mbalassi and @zentol for the review.
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2299
I think it would be fine to merge it without a test; we merged plenty of PR's like this without requiring a test.
The PythonPlanBinderTest failing is odd, but i would guess that it is unrelated to this change. Nevertheless, does it get stuck every time you execute it? And, of course, does it no longer fail without this commit?
---
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] flink issue #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileCache#co...
Posted by mbalassi <gi...@git.apache.org>.
Github user mbalassi commented on the issue:
https://github.com/apache/flink/pull/2299
Thanks @nssalian, merging then.
---
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] flink pull request #2299: [FLINK-4259]: Unclosed FSDataOutputStream in FileC...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2299
---
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.
---