You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tillrohrmann <gi...@git.apache.org> on 2018/03/21 20:20:49 UTC
[GitHub] flink pull request #5740: [FLINK-9027] [web] Clean up web UI resources by in...
GitHub user tillrohrmann opened a pull request:
https://github.com/apache/flink/pull/5740
[FLINK-9027] [web] Clean up web UI resources by installing shut down hook
## What is the purpose of the change
The RestServerEndpoint creates temp directories to upload files and store the web ui.
In case of a hard shut down, as it happens with bin/stop-cluster.sh we should still
delete these files. We do this by installing a shut down hook.
## Verifying this change
- Manually tested
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tillrohrmann/flink cleanupWebUI
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5740.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 #5740
----
commit 719abe3766f406e5b0cac4afc3597a084061769c
Author: Till Rohrmann <tr...@...>
Date: 2018-03-21T20:04:40Z
[FLINK-9027] [web] Clean up web UI resources by installing shut down hook
The RestServerEndpoint creates temp directories to upload files and store the web ui.
In case of a hard shut down, as it happens with bin/stop-cluster.sh we should still
delete these files. We do this by installing a shut down hook.
commit 659479f6d67b97bf63b66df57eeaf5c11195af73
Author: Till Rohrmann <tr...@...>
Date: 2018-03-21T20:08:13Z
[hotfix] Make RestServerEndpoint#uploadDir protected
----
---
[GitHub] flink issue #5740: [FLINK-9027] [web] Clean up web UI resources by installin...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/5740
Thanks for the feedback @StephanEwen. Merging this PR.
---
[GitHub] flink issue #5740: [FLINK-9027] [web] Clean up web UI resources by installin...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/5740
I agree that having a single shut down hook for cleaning up the temp directory is the best solution. I think that the `TaskManager` does not properly do it like you've described. The `TaskExecutorLocalStateStoresManager` registers its own shut down hooks and most of the `TaskManager` components like `BlobCache`, `FileCache` and `IOManager` do so likewise.
The upload directory is a good point. With my changes it will always be deleted. If this is not the intended behaviour, then I will adapt it accordingly.
In order to not widen the scope of this task too much I would suggest the following:
1. Addressing the general problem of reducing the number of shut down hooks in a separate issue
2. Creating a a sub directory in `WebOptions#TMP_DIR` which is set up and cleaned up by the `ClusterEntrypoint` and its shut down hook
What do you think @StephanEwen.
---
[GitHub] flink pull request #5740: [FLINK-9027] [web] Clean up web UI resources by in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5740
---
[GitHub] flink issue #5740: [FLINK-9027] [web] Clean up web UI resources by installin...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/5740
Sounds like a good idea.
---
[GitHub] flink issue #5740: [FLINK-9027] [web] Clean up web UI resources by installin...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5740
Okay, seems like the TaskManager design diverged from the "single temp dir" assumption we had originally.
How about creating the upload dir inside the web temp dir by default (then it is deleted), but not delete it explicitly (in which case it stays if users explicitly configure it to a different location)?
---
[GitHub] flink issue #5740: [FLINK-9027] [web] Clean up web UI resources by installin...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5740
The TaskManagers have one (or more) root directories for all temp files. They only have one shutdown hook that removes them at the very end. No separate hooks in the separate components.
Should we try and have the same in the JobManager?
Aside form that, I think previously users liked that the jar upload directory was not cleaned by default when they explicitly configured it. Does this change preserve the behavior?
---