[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-130588652 Thanks for your changes. I think we should use `read()` instead of `readLine()` because we are using a custom delimiter and not necessarily \n (newline symbol). The danger

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r37748868 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: Framesize fix

2015-08-24 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/934#issuecomment-134200054 I see the point in keeping the Akka message-based transport of accumulators. @kl0u Sorry, I was also on vacations. Could you rebase to the current master again? Then, I

[GitHub] flink pull request: [scripts] resolve base path of symlinked execu...

2015-08-24 Thread mxm
GitHub user mxm opened a pull request: https://github.com/apache/flink/pull/1049 [scripts] resolve base path of symlinked executable This bootstraps Flink from a symlinked bin/flink executable. It's a special case but IMHO worthwhile to consider. You can merge this pull request

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r37749287 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-134199343 @HuangWHWHW Sorry for keeping you waiting. I've made some more comments. Otherwise, I think this looks ready to be merged. --- If your project is set up for it, you can

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r37749598 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [scripts] resolve base path of symlinked execu...

2015-08-24 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/1049#issuecomment-134192838 Unfortunately, we cannot access the config.sh file in this case. So code reuse is not possible. I've also seen the code you posted. It looks awfully hacky and we could

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r37748844 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r37748808 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [scripts] resolve base path of symlinked execu...

2015-08-24 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/1049#issuecomment-134230260 Yes, the two are in sync now. --- 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-2480][test]Add tests for PrintSinkFunct...

2015-08-24 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/991#discussion_r37762004 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java --- @@ -0,0 +1,234

[GitHub] flink pull request: [scripts] resolve base path of symlinked execu...

2015-08-24 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/1049#issuecomment-134298584 Apparently, the `readlink` utility is not part of POSIX. So we might have to revert to the old way of parsing the output of `ls -ld` to stay compatible. It is actually more

[GitHub] flink pull request: Cascading changes for compatibility

2015-07-29 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/950#issuecomment-125988920 Yes, I've updated the pull request. --- 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

[GitHub] flink pull request: [FLINK-2404]Primitive add methods for Accumula...

2015-07-28 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/942#issuecomment-125504599 Thanks for repeating your experiment isolated without the whole Flink stack. Here we can clearly see the performance impact of the `Long` auto unboxing. I

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635357 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java --- @@ -309,6 +315,32 @@ public Task(TaskDeploymentDescriptor tdd

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635351 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java --- @@ -309,6 +315,32 @@ public Task(TaskDeploymentDescriptor tdd

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635338 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java --- @@ -259,6 +261,10 @@ public Task(TaskDeploymentDescriptor tdd

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635367 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/SerializedValue.java --- @@ -57,6 +60,25 @@ public T deserializeValue(ClassLoader loader

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635383 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/akka/AkkaUtils.scala --- @@ -421,4 +421,16 @@ object AkkaUtils { val duration = Duration

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635395 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala --- @@ -338,16 +332,51 @@ class JobManager

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635361 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/SerializedValue.java --- @@ -47,6 +47,9 @@ public SerializedValue(T value) throws IOException

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635387 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/akka/AkkaUtils.scala --- @@ -421,4 +421,16 @@ object AkkaUtils { val duration = Duration

[GitHub] flink pull request: [FLINK-2404]Primitive add methods for Accumula...

2015-07-28 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/942#issuecomment-125603184 We can add a comment that the new ones are faster on primitives. It is not possible to deprecate the older ones because they are inherited from the Accumulator interface

[GitHub] flink pull request: [FLINK-1819][core]Allow access to RuntimeConte...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/966#issuecomment-126705428 Thanks for the pull request! You did not convert all InputFormats and OutputFormats. There are a few more, e.g. HadoopInputFormatBase or JDBCInputFormat. Other than

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126674684 Thank you for your patience :) I will merge this once the tests have passed. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991461 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -893,13 +1086,11 @@ public Object call() throws

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991960 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -560,53 +621,177 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991476 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -979,8 +1174,7 @@ public void scheduleOrUpdateConsumers

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991467 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -918,8 +1109,7 @@ private void postRunCleanup

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991472 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -966,8 +1162,7 @@ public boolean updateState

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991266 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -75,24 +78,24 @@ * The execution graph

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991305 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -560,53 +621,177 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35991400 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -619,6 +718,21 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/934#issuecomment-126752412 Hi @kl0u , Thanks for addressing my comments. I've made some new ones :) but looks much better. As for merging the different types of snapshots, the only

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/957#discussion_r35966826 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendLoggingTest.java --- @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/957#discussion_r35967492 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendLoggingTest.java --- @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software

[GitHub] flink pull request: [FLINK-2404]Primitive add methods for Accumula...

2015-07-28 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/942#issuecomment-125652591 For the cases, where we add a primitive type on a Long/Int/DoubleCounter, the optimization will automatically be used. --- If your project is set up for it, you can reply

[GitHub] flink pull request: [FLINK-2404]Primitive add methods for Accumula...

2015-07-28 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/942#issuecomment-125653547 Thanks for the pull request! --- 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: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635236 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FallbackLibraryCacheManager.java --- @@ -33,6 +33,11 @@ private

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634947 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/LargeAccumulatorSnapshot.java --- @@ -0,0 +1,54 @@ +/* + * Licensed

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634991 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/client/SerializedJobExecutionResult.java --- @@ -38,7 +42,13 @@ private final JobID

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634697 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/BaseAccumulatorSnapshot.java --- @@ -27,12 +27,7 @@ import java.io.Serializable

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634723 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/LargeAccumulatorHelper.java --- @@ -0,0 +1,125 @@ +/* + * Licensed

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635490 --- Diff: flink-runtime/src/test/scala/org/apache/flink/runtime/testingUtils/TestingJobManager.scala --- @@ -181,7 +181,7 @@ trait TestingJobManager extends

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/934#issuecomment-125561321 All in all, nice work @kl0u. Do you think we could change this pull request such that we can handle small/large or message/blobcached accumulators more transparently

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634952 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/LargeAccumulatorSnapshot.java --- @@ -0,0 +1,54 @@ +/* + * Licensed

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634886 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/BaseAccumulatorSnapshot.java --- @@ -44,18 +39,11 @@ */ private

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35634978 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/SmallAccumulatorSnapshot.java --- @@ -0,0 +1,50 @@ +/* + * Licensed

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635294 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -560,35 +572,118 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635322 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -619,6 +718,21 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635248 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -142,17 +146,25 @@ * through

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635297 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -599,14 +694,18 @@ public ExecutionContext

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635471 --- Diff: flink-tests/src/test/java/org/apache/flink/test/misc/MiscellaneousIssuesITCase.java --- @@ -174,4 +177,52 @@ public void flatMap(Long value

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635421 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala --- @@ -911,6 +953,8 @@ class JobManager( try

[GitHub] flink pull request: Framesize fix

2015-07-28 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/934#discussion_r35635418 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala --- @@ -516,6 +545,19 @@ class JobManager

[GitHub] flink pull request: Cascading changes for compatibility

2015-07-29 Thread mxm
GitHub user mxm opened a pull request: https://github.com/apache/flink/pull/950 Cascading changes for compatibility @fhueske and me are working on getting Cascading to run on top of Flink. These two commits introduce changes that were necessary to make the translation possible

[GitHub] flink pull request: [FLINK-2387] add streaming test case for live ...

2015-07-29 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/926#issuecomment-125955678 Sorry for the plain description. This pull request adds a test for the streaming part of the live accumulators, i.e. it makes sure that user-defined and Flink internal

[GitHub] flink pull request: [FLINK-2387] add streaming test case for live ...

2015-08-05 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/926#issuecomment-128002553 Alright, then let me also rebase and investigate the error. It might be some regression due to rebasing. --- If your project is set up for it, you can reply to this email

[GitHub] flink pull request: [FLINK-1927][py] Operator distribution rework

2015-07-30 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/931#issuecomment-126218526 I think we can merge this later on. --- 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

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126612061 Thanks for adding the tests! This looks good to merge. Could you update https://ci.apache.org/projects/flink/flink-docs-master/apis/cli.html ? --- If your project is set up

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126620784 Thanks. One more thing. Could you also update https://ci.apache.org/projects/flink/flink-docs-release-0.9/setup/config.html ? You introduced a new config entry

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/957#discussion_r35956433 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendLoggingTest.java --- @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126625365 Thank you! Merging. --- 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-2248][client]Add flag to disable sysout...

2015-07-31 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126631326 Actually, I'm not sure about the config entry. We already have a lot of config entries and I don't really see the purpose of making print output of the client a system wide

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-30 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/957#discussion_r35855355 --- Diff: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java --- @@ -51,6 +51,9 @@ The parallelism with which

[GitHub] flink pull request: [FLINK-2248][client]Add flag to disable sysout...

2015-07-30 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/957#issuecomment-126254186 Thanks for the pull request. Could you add a test case to check whether the output is suppressed when `-q` is provided? Also, it would be great if you could update

[GitHub] flink pull request: [FLINK-1927][py] Operator distribution rework

2015-07-29 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/931#issuecomment-125979125 Thanks for the pull request @zentol! +1 for removing the dill library. As far as I can see, we handle all the serialization ourselves now. We only used the Dill

[GitHub] flink pull request: [FLINK-1927][py] Operator distribution rework

2015-07-29 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/931#discussion_r35763245 --- Diff: flink-staging/flink-language-binding/flink-python/src/main/java/org/apache/flink/languagebinding/api/java/python/streaming/PythonStreamer.java

[GitHub] flink pull request: [FLINK-1819][core]Allow access to RuntimeConte...

2015-08-11 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/966#issuecomment-129849477 Yes, existing formats should converted to rich formats. The name `Abstract..` makes more sense if it becomes the new default way of interfacing with the rich input/output

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-11 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-129864531 @HuangWHWHW `retryForever` is just a convenience variable for `maxRetry 0`. Your fix is correct because the loop will only execute if `maxRetry 0` and thus not execute

[GitHub] flink pull request: [FLINK-1819][core]Allow access to RuntimeConte...

2015-08-11 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/966#issuecomment-129826022 @StephanEwen @fhueske The `Rich` prefix might seem a bit odd but it is a naming convention that is consistent with the user-defined functions which have

[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...

2015-08-11 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/1008#issuecomment-129947817 This is great and will be very helpful while debugging! Only the output is very hard to read. How about this? Just some new lines and indention: ``` Cannot load user

[GitHub] flink pull request: [FLINK-2432] Custom serializer support

2015-08-07 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/962#discussion_r36502940 --- Diff: flink-staging/flink-language-binding/flink-python/src/test/python/org/apache/flink/languagebinding/api/python/flink/test/test_custom.py --- @@ -0,0

[GitHub] flink pull request: User defined communication between parallel in...

2015-08-07 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/975#issuecomment-128664224 Fabian is right: This is a major change in the computation paradigm of Flink which can cause all kinds of side effects when executing user-defined functions on the task

[GitHub] flink pull request: [FLINK-2495][fix]Add a null point check in API...

2015-08-07 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/999#discussion_r36508794 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/datastream/DataStream.java --- @@ -256,9 +256,11

[GitHub] flink pull request: [FLINK-2494 ]Fix StreamGraph getJobGraph bug

2015-08-07 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/998#issuecomment-128670714 @ffbin If the issue has been resolved, could you please close this pull request? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request: [FLINK-2437] Fix default constructor detection...

2015-08-07 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/960#issuecomment-128662225 Stephan is right. The changed messages just add noise to the pull request which makes it harder to review. --- If your project is set up for it, you can reply to this email

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r36973279 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r36973254 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunctionTest.java

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r36973430 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunction.java

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/992#discussion_r36973377 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunction.java

[GitHub] flink pull request: [FLINK-2480][test]Add tests for PrintSinkFunct...

2015-08-13 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/991#discussion_r36990804 --- Diff: flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java --- @@ -0,0 +1,234

[GitHub] flink pull request: [FLINK-1819][core]Allow access to RuntimeConte...

2015-08-13 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/966#issuecomment-130677380 Thanks for the contribution @sachingoel0101! --- 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

[GitHub] flink pull request: [FLINK-2437] Fix default constructor detection...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/960#issuecomment-130219402 Your changes look good to merge. --- 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

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-130213092 @HuangWHWHW `read()` method of the `BufferedReader` object returns `-1` in case the end of the stream has been reached. A couple of things I noticed apart from

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-130222078 Actually point 3 is not so bad because we're using a buffered reader that fills the buffer and does not read a character from the socket on every call to `read

[GitHub] flink pull request: [FLINK-2437] Fix default constructor detection...

2015-08-12 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/960#discussion_r36837638 --- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java --- @@ -1328,24 +1329,29 @@ else if(typeHierarchy.size() = 1

[GitHub] flink pull request: [FLINK-2437] Fix default constructor detection...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/960#issuecomment-130251642 Thanks for your contribution! --- 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-2490][FIX]Remove the retryForever check...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-130237642 1.In using StringBuilder, does it mean that we should use BufferedReader.readLine() instead of BufferedReader.read()? Reading by character is the way to go if we

[GitHub] flink pull request: [FLINK-2480][test]Add tests for PrintSinkFunct...

2015-08-12 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/991#issuecomment-130295740 Your pull request doesn't compile: https://s3.amazonaws.com/archive.travis-ci.org/jobs/74504427/log.txt --- If your project is set up for it, you can reply to this email

[GitHub] flink pull request: [FLINK-2387] add streaming test case for live ...

2015-08-10 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/926#issuecomment-129378149 I changed the streaming test to reuse the batch test's logic. Should be good to go know. Merging later on. --- If your project is set up for it, you can reply to this email

[GitHub] flink pull request: [FLINK-2500][Streaming]remove a unwanted if ...

2015-08-10 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/1001#issuecomment-129328760 Your changes will have very little impact on the performance. However, they are semantically correct and can be merged. --- If your project is set up for it, you can reply

[GitHub] flink pull request: [FLINK-2387] add streaming test case for live ...

2015-08-10 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/926#issuecomment-129460830 Yes, exactly :) --- 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

[GitHub] flink pull request: [FLINK-2387] add streaming test case for live ...

2015-08-10 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/926#issuecomment-129414456 Yes, it was a race condition that occurred only with the previous streaming test design. The synchronization wouldn't always ensure that the SourceTask had been brought up

[GitHub] flink pull request: [FLINK-2432] Custom serializer support

2015-08-06 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/962#discussion_r36394313 --- Diff: flink-staging/flink-language-binding/flink-python/src/test/python/org/apache/flink/languagebinding/api/python/flink/test/test_custom.py --- @@ -0,0

[GitHub] flink pull request: [FLINK-2490][FIX]Remove the retryForever check...

2015-08-13 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/992#issuecomment-130672880 Otherwise, I found the SocketClientSink didn`t have the retry. Is it necessary to get a retry? Yes, that might be an issue but let's keep it separate from our

[GitHub] flink pull request: [FLINK-2097] Implement job session management

2015-07-22 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/858#issuecomment-123755792 It should just add more nodes to the ExecutionGraph. Existing ones should not be modified. For batch, I think the assumption is that it needs to be finished. For streaming

[GitHub] flink pull request: Collect(): Fixing the akka.framesize size limi...

2015-07-22 Thread mxm
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/887#issuecomment-123756678 @kl0u Sure, I've sent you an email. --- 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

[GitHub] flink pull request: [FLINK-1818] Added api to cancel job from clie...

2015-07-22 Thread mxm
Github user mxm commented on a diff in the pull request: https://github.com/apache/flink/pull/642#discussion_r35240445 --- Diff: flink-clients/src/main/java/org/apache/flink/client/program/Client.java --- @@ -421,6 +426,50 @@ public JobSubmissionResult run(JobGraph jobGraph

<    1   2   3   4   5   6   7   8   9   10   >