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/07/13 11:11:21 UTC
[GitHub] flink pull request #6329: [FLINK-9841] Web UI only show partial taskmanager ...
GitHub user yanghua opened a pull request:
https://github.com/apache/flink/pull/6329
[FLINK-9841] Web UI only show partial taskmanager log
## What is the purpose of the change
*This pull request fixed a bug triggered web UI only show partial taskmanager log*
## Brief change log
- *Remove the redundant resource close*
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## 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-9841
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6329.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 #6329
----
commit e69efdef7546bd88c5a73d303e689ea5d051b931
Author: yanghua <ya...@...>
Date: 2018-07-13T08:48:04Z
[FLINK-9841] Web UI only show partial taskmanager log
----
---
[GitHub] flink pull request #6329: [FLINK-9841] Web UI only show partial taskmanager ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/6329
---
[GitHub] flink issue #6329: [FLINK-9841] Web UI only show partial taskmanager log
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on the issue:
https://github.com/apache/flink/pull/6329
@zentol yes, you are right, sorry about my expression. here we should not use try-with-resource, because the listener will close the file. And it seems try-with-resource close operation more faster than the complete listener.
---
[GitHub] flink pull request #6329: [FLINK-9841] Web UI only show partial taskmanager ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6329#discussion_r202655436
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/taskmanager/AbstractTaskManagerFileHandler.java ---
@@ -208,51 +209,77 @@ private void removeBlob(RemovalNotification<ResourceID, CompletableFuture<Transi
}
private void transferFile(ChannelHandlerContext ctx, File file, HttpRequest httpRequest) throws FlinkException {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(file, "r")) {
- final long fileLength = randomAccessFile.length();
+ final RandomAccessFile randomAccessFile;
- try (final FileChannel fileChannel = randomAccessFile.getChannel()) {
+ try {
+ randomAccessFile = new RandomAccessFile(file, "r");
+ } catch (FileNotFoundException e) {
+ throw new FlinkException("Can not find file " + file + ".", e);
+ }
- HttpResponse response = new DefaultHttpResponse(HTTP_1_1, OK);
- response.headers().set(CONTENT_TYPE, "text/plain");
+ final long fileLength;
- if (HttpHeaders.isKeepAlive(httpRequest)) {
- response.headers().set(CONNECTION, HttpHeaders.Values.KEEP_ALIVE);
- }
- HttpHeaders.setContentLength(response, fileLength);
-
- // write the initial line and the header.
- ctx.write(response);
-
- // write the content.
- final ChannelFuture lastContentFuture;
- final GenericFutureListener<Future<? super Void>> completionListener = future -> {
- fileChannel.close();
- randomAccessFile.close();
- };
-
- if (ctx.pipeline().get(SslHandler.class) == null) {
- ctx.write(
- new DefaultFileRegion(fileChannel, 0, fileLength), ctx.newProgressivePromise())
- .addListener(completionListener);
- lastContentFuture = ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT);
-
- } else {
- lastContentFuture = ctx
- .writeAndFlush(
- new HttpChunkedInput(new ChunkedFile(randomAccessFile, 0, fileLength, 8192)),
- ctx.newProgressivePromise())
- .addListener(completionListener);
-
- // HttpChunkedInput will write the end marker (LastHttpContent) for us.
- }
+ try {
+ fileLength = randomAccessFile.length();
--- End diff --
you could move this back into the following try block, like this:
```
try {
final long fileLength = randomAccessFile.length();
final FileChannel fileChannel = randomAccessFile.getChannel();
try {
[... actual handler code ...]
} catch (Exception e) {
fileChannel.close();
throw e;
}
} catch (IOException ioe) {
try {
randomAccessFile.close();
} catch (IOException e) {
log.warn("Error while closing file.", e);
}
}
```
---
[GitHub] flink issue #6329: [FLINK-9841] Web UI only show partial taskmanager log
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on the issue:
https://github.com/apache/flink/pull/6329
@zentol this PR match your requirement? I hope it can be merged into 1.6, so that users can see the full taskmanager log.
---
[GitHub] flink issue #6329: [FLINK-9841] Web UI only show partial taskmanager log
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on the issue:
https://github.com/apache/flink/pull/6329
@dawidwys can you review this PR?
---