You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2017/05/14 20:04:09 UTC
[GitHub] flink pull request #3896: [FLINK-6370] [webUI] Handle races for single file ...
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/3896
[FLINK-6370] [webUI] Handle races for single file in FileServerHandler
This PR prevents a race between multiple requests trying to create the same file, which previously would cause one request to fail.
We can't just ignore the `FileAlreadyExistsException` since there's no guarantee that the file copying is complete. To prevent this scenario the copying is now done in a `synchronized` block, along with ignoring the `FileAlreadyExistsExceptions`.
Due to the small size of files loaded from the class-loader this should have no impact on the responsiveness of the webUI.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 6370_web_file
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3896.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 #3896
----
commit 4ebee58fa8c39d0aa6d68d1470ea3a850e92b954
Author: zentol <ch...@apache.org>
Date: 2017-05-14T19:57:16Z
[FLINK-6370] [webUI] Handle races for single file in FileServerHandler
----
---
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 #3896: [FLINK-6370] [webUI] Handle races for single file ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3896#discussion_r116500253
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/files/StaticFileServerHandler.java ---
@@ -234,8 +237,17 @@ private void respondAsLeader(ChannelHandlerContext ctx, HttpRequest request, Str
if (!rootURI.relativize(requestedURI).equals(requestedURI)) {
logger.debug("Loading missing file from classloader: {}", requestPath);
// ensure that directory to file exists.
- file.getParentFile().mkdirs();
- Files.copy(resourceStream, file.toPath());
+ if (!file.getParentFile().mkdirs()) {
+ throw new IOException("Could not create directories for file " + file);
+ }
+ synchronized (COPY_LOCK) {
--- End diff --
There is nothing stopping us from changing to eager loading later on, so I opted for the fastest, yet still reasonable, fix for the problem.
---
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 #3896: [FLINK-6370] [webUI] Handle races for single file ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol closed the pull request at:
https://github.com/apache/flink/pull/3896
---
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 #3896: [FLINK-6370] [webUI] Handle races for single file ...
Posted by dernasherbrezon <gi...@git.apache.org>.
Github user dernasherbrezon commented on a diff in the pull request:
https://github.com/apache/flink/pull/3896#discussion_r116474287
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/files/StaticFileServerHandler.java ---
@@ -234,8 +237,17 @@ private void respondAsLeader(ChannelHandlerContext ctx, HttpRequest request, Str
if (!rootURI.relativize(requestedURI).equals(requestedURI)) {
logger.debug("Loading missing file from classloader: {}", requestPath);
// ensure that directory to file exists.
- file.getParentFile().mkdirs();
- Files.copy(resourceStream, file.toPath());
+ if (!file.getParentFile().mkdirs()) {
+ throw new IOException("Could not create directories for file " + file);
+ }
+ synchronized (COPY_LOCK) {
--- End diff --
Looks like workaround. Why not simply extract all static resources on startup?
---
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 #3896: [FLINK-6370] [webUI] Handle races for single file ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3896#discussion_r116500027
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/files/StaticFileServerHandler.java ---
@@ -234,8 +237,17 @@ private void respondAsLeader(ChannelHandlerContext ctx, HttpRequest request, Str
if (!rootURI.relativize(requestedURI).equals(requestedURI)) {
logger.debug("Loading missing file from classloader: {}", requestPath);
// ensure that directory to file exists.
- file.getParentFile().mkdirs();
- Files.copy(resourceStream, file.toPath());
+ if (!file.getParentFile().mkdirs()) {
+ throw new IOException("Could not create directories for file " + file);
+ }
+ synchronized (COPY_LOCK) {
--- End diff --
Because this was easy to write, is easy to review, does not change any behavior and isn't particularly intrusive.
I agree that we should have a discussion as to whether we should load the files lazily or not, but not now when the next release is coming up and everyone is scrambling to fix the most issues in as little time as possible
---
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.
---