Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1048
---
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
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135746375
Actually, we can somewhat ignore the first problem - it seems that we can
rebase the client refactoring pull requests well onto this.
---
If your project is set up
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135746761
Okay, one change I would like to do during merging is to not deserialize
exceptions on the JobManager. I would extend the SerializedThrowable to include
the
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135770745
The `SerializedThrowable` is then a subclass of `Exception`?
I think it's a very good solution to not deserialize the exception on the
`JobManager`.
---
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135767450
I agree. In both cases the compiler cannot help much to avoid forgetting
about it and, thus, it's the responsibility of the programmer to make sure that
the
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135769797
The added advantage of that approach s that we keep no user exceptions
alive in the execution graph.
Not keeping those alive is quite desirable, actually,
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135769165
I have now actually a pretty interesting solution. I changed the
SerializedThrowable such that it looks like the original exception, with
respect to message, stack
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135765919
Explicit would mean here to send something where the receiver knows that he
has to deserialize. Whether through a dedicated message, or a
SerializedThrowable that
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135770247
That sounds very good.
---
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
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135811858
I'm not sure whether this would solve the initial problem. I thought the
problem was that a user code exception can be sent from the `JobManager` to the
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135793882
Only disadvantage is that archived execution graphs prevent user class
unloading...
---
If your project is set up for it, you can reply to this email and have your
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135812417
Sending a stringified exception is not an option between jm and client
because the user might rely on the exception in the RemoteExecEnv.
---
If your project is set
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135793797
Hmm... the more I work on it, the trickier it appears.
It is efficient when done rigth, but one needs to watch carefully at what
places to wrap an exception
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135813751
Right, I am going for an explicit serialized form of the exception at this
point.
---
If your project is set up for it, you can reply to this email and have your
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078466
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/minicluster/FlinkMiniCluster.scala
---
@@ -239,7 +239,7 @@ abstract class
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078400
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -349,27 +351,29 @@ class JobManager(
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078531
--- Diff: flink-tests/src/test/resources/log4j-test.properties ---
@@ -18,7 +18,7 @@
# Set root logger level to OFF to not flood build logs
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078839
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -349,27 +351,29 @@ class JobManager(
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135367041
I had some more comments.
---
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
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078804
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -349,27 +351,29 @@ class JobManager(
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-135363969
I addressed all PR comments.
---
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
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078076
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/client/JobClient.java ---
@@ -177,9 +176,18 @@ public static SerializedJobExecutionResult
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r38078252
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -349,27 +351,29 @@ class JobManager(
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37754281
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/util/SerializedThrowable.java
---
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37754479
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -327,8 +327,11 @@ class JobManager(
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37749500
--- Diff:
flink-clients/src/main/java/org/apache/flink/client/LocalExecutor.java ---
@@ -176,8 +175,7 @@ public JobExecutionResult executePlan(Plan plan)
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37753243
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/client/JobClient.java ---
@@ -123,12 +122,12 @@ public static InetSocketAddress
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37753621
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java
---
@@ -1028,8 +1029,12 @@ public void
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-134206474
Looks good, modulo some minor comments. But maybe @tillrohrmann has
something to say, let's wait for him.
---
If your project is set up for it, you can reply to this
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37752056
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/util/SerializedThrowable.java
---
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache
GitHub user rmetzger opened a pull request:
https://github.com/apache/flink/pull/1048
[FLINK-2543] Fix user object deserialization for user state
File-based state handles were using the system classloader to deserialize
the state object.
Exceptions send from the JobManager
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37753999
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/TaskExecutionState.java
---
@@ -160,19 +123,10 @@ public
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37756350
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/client/JobClientActor.java
---
@@ -128,6 +128,13 @@ else if (message instanceof
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1048#issuecomment-134226512
I had some comments. The main question is why do we introduce a new
`JobFailure` message instead of treating a `SerializedThrowable` as what it is,
namely a
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37754792
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/messages/ExecutionGraphMessages.scala
---
@@ -80,7 +81,7 @@ object
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1048#discussion_r37756262
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/client/JobClient.java ---
@@ -160,12 +159,12 @@ public static
36 matches
Mail list logo