[GitHub] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-30 Thread asfgit
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread rmetzger
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread rmetzger
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-28 Thread StephanEwen
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread rmetzger
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-27 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread aljoscha
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread aljoscha
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread aljoscha
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread rmetzger
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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] flink pull request: [FLINK-2543] Fix user object deserialization f...

2015-08-24 Thread tillrohrmann
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