You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by yanghua <gi...@git.apache.org> on 2018/03/20 02:08:32 UTC
[GitHub] flink pull request #5723: [FLINK-9019] Unclosed closeableRegistry in StreamT...
GitHub user yanghua opened a pull request:
https://github.com/apache/flink/pull/5723
[FLINK-9019] Unclosed closeableRegistry in StreamTaskStateInitializerImpl#rawOperatorStateInputs
## What is the purpose of the change
*This pull request fixed a resource(CloseableRegistry) leak.*
## Brief change log
- *create the `ClosableRegistry`'s instance if necessary*
## Verifying this change
This change is already covered by existing tests, such as *StreamTaskStateInitializerImplTest*.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / **not documented**)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yanghua/flink FLINK-9019
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5723.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 #5723
----
commit 447c4a56b43d3c0737ad54e4e91846ea54ff7205
Author: yanghua <ya...@...>
Date: 2018-03-20T02:02:21Z
[FLINK-9019] Unclosed closeableRegistry in StreamTaskStateInitializerImpl#rawOperatorStateInputs
----
---
[GitHub] flink pull request #5723: [FLINK-9019] Unclosed closeableRegistry in StreamT...
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on a diff in the pull request:
https://github.com/apache/flink/pull/5723#discussion_r175729909
--- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamTaskStateInitializerImpl.java ---
@@ -279,15 +279,14 @@ protected OperatorStateBackend operatorStateBackend(
if (restoreStateAlternatives.hasNext()) {
- final CloseableRegistry closeableRegistry = new CloseableRegistry();
-
Collection<OperatorStateHandle> rawOperatorState = restoreStateAlternatives.next();
// TODO currently this does not support local state recovery, so we expect there is only one handle.
Preconditions.checkState(
!restoreStateAlternatives.hasNext(),
"Local recovery is currently not implemented for raw operator state, but found state alternative.");
if (rawOperatorState != null) {
+ final CloseableRegistry closeableRegistry = new CloseableRegistry();
--- End diff --
Yes. Obviously, we could do this. Let me refactor it.
---
[GitHub] flink pull request #5723: [FLINK-9019] Unclosed closeableRegistry in StreamT...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5723
---
[GitHub] flink issue #5723: [FLINK-9019] Unclosed closeableRegistry in StreamTaskStat...
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on the issue:
https://github.com/apache/flink/pull/5723
cc @tillrohrmann
---
[GitHub] flink issue #5723: [FLINK-9019] Unclosed closeableRegistry in StreamTaskStat...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/5723
Thanks for addressing my comments. Merging this PR once Travis gives green light.
---
[GitHub] flink issue #5723: [FLINK-9019] Unclosed closeableRegistry in StreamTaskStat...
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on the issue:
https://github.com/apache/flink/pull/5723
cc @tillrohrmann
---
[GitHub] flink pull request #5723: [FLINK-9019] Unclosed closeableRegistry in StreamT...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/5723#discussion_r175705138
--- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamTaskStateInitializerImpl.java ---
@@ -279,15 +279,14 @@ protected OperatorStateBackend operatorStateBackend(
if (restoreStateAlternatives.hasNext()) {
- final CloseableRegistry closeableRegistry = new CloseableRegistry();
-
Collection<OperatorStateHandle> rawOperatorState = restoreStateAlternatives.next();
// TODO currently this does not support local state recovery, so we expect there is only one handle.
Preconditions.checkState(
!restoreStateAlternatives.hasNext(),
"Local recovery is currently not implemented for raw operator state, but found state alternative.");
if (rawOperatorState != null) {
+ final CloseableRegistry closeableRegistry = new CloseableRegistry();
--- End diff --
Can't we even move this registry into the anonymous class?
---