You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by NicoK <gi...@git.apache.org> on 2018/07/10 08:11:08 UTC
[GitHub] flink pull request #6291: [FLINK-9785][network] add remote address informati...
GitHub user NicoK opened a pull request:
https://github.com/apache/flink/pull/6291
[FLINK-9785][network] add remote address information to LocalTransportException instances
## What is the purpose of the change
In contrast to the messages inside `RemoteTransportException`s, `LocalTransportException` instances did not contain information about the remote end which complicated debugging and correlating different log files.
## Brief change log
- add remote address information to `LocalTransportException` instantiations
## 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): **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/NicoK/flink flink-9785
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6291.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 #6291
----
commit ada95a5eec12e07981cbc99f5530d3f0775b1daa
Author: Nico Kruber <ni...@...>
Date: 2018-07-09T14:49:15Z
[FLINK-9785][network] add remote address information to LocalTransportException instances
----
---
[GitHub] flink issue #6291: [FLINK-9785][network] add remote address information to L...
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on the issue:
https://github.com/apache/flink/pull/6291
merging...
---
[GitHub] flink issue #6291: [FLINK-9785][network] add remote address information to L...
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on the issue:
https://github.com/apache/flink/pull/6291
@yanghua sure, why not ... although there's no performance problem in string concatenation for exceptions - we'll abort anyway
---
[GitHub] flink pull request #6291: [FLINK-9785][network] add remote address informati...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/6291
---
[GitHub] flink pull request #6291: [FLINK-9785][network] add remote address informati...
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on a diff in the pull request:
https://github.com/apache/flink/pull/6291#discussion_r201304267
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientHandler.java ---
@@ -164,7 +164,9 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
+ "that the remote task manager was lost.", remoteAddr, cause);
}
else {
- tex = new LocalTransportException(cause.getMessage(), ctx.channel().localAddress(), cause);
+ SocketAddress localAddr = ctx.channel().localAddress();
+ tex = new LocalTransportException(cause.getMessage() + " (connection to '" + remoteAddr + "')",
--- End diff --
the same
---
[GitHub] flink pull request #6291: [FLINK-9785][network] add remote address informati...
Posted by yanghua <gi...@git.apache.org>.
Github user yanghua commented on a diff in the pull request:
https://github.com/apache/flink/pull/6291#discussion_r201303678
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/CreditBasedPartitionRequestClientHandler.java ---
@@ -167,7 +167,8 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
tex = new RemoteTransportException("Lost connection to task manager '" + remoteAddr + "'. " +
"This indicates that the remote task manager was lost.", remoteAddr, cause);
} else {
- tex = new LocalTransportException(cause.getMessage(), ctx.channel().localAddress(), cause);
+ final SocketAddress localAddr = ctx.channel().localAddress();
+ tex = new LocalTransportException(cause.getMessage() + " (connection to '" + remoteAddr + "')", localAddr, cause);
--- End diff --
using `String.format` here looks better to me
---