[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46278355 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ExecutionVertexStopTest.java --- @@ -0,0 +1,138 @@ +/* + *

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46283023 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ExecutionVertexStopTest.java --- @@ -0,0 +1,138 @@ +/* + * Licensed to

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46282619 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskStopTest.java --- @@ -0,0 +1,113 @@ +/* + * Licensed to the

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46299763 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskStopTest.java --- @@ -0,0 +1,113 @@ +/* + * Licensed to the

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46300103 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerTest.java --- @@ -227,4 +235,121 @@ public void

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46300384 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskStopTest.java --- @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-12-01 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46300626 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerTest.java --- @@ -227,4 +235,121 @@ public void testRequestPartitionState()

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46205018 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -569,6 +571,69 @@ public int compare(JobStatusMessage o1,

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46204977 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -569,6 +571,69 @@ public int compare(JobStatusMessage o1,

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-160779157 Same failing test as before. We forgot to address this: "I just investigated the failing test. The test is new and passed up to know. @uce changed ExecutionGraph.cancel()

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46205386 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendStopTest.java --- @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-160774595 Updated this PR. --- 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-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46205848 --- Diff: flink-runtime-web/web-dashboard/package.json --- @@ -7,27 +7,27 @@ "devDependencies": { "browserify": "^9.0.3", "coffeeify":

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46205569 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendStopTest.java --- @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46206488 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java --- @@ -402,21 +405,53 @@ public void onComplete(Throwable

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46171068 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendStopTest.java --- @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46171614 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java --- @@ -188,9 +189,12 @@ public WebRuntimeMonitor(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46172044 --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.svc.coffee --- @@ -216,4 +216,7 @@ angular.module('flinkApp') # proper

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46173267 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -1098,8 +1132,8 @@ public void

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46172301 --- Diff: flink-runtime-web/web-dashboard/package.json --- @@ -7,27 +7,27 @@ "devDependencies": { "browserify": "^9.0.3",

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46173139 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java --- @@ -254,13 +262,14 @@ public ExecutionGraph(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46174663 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/messages/TaskControlMessages.scala --- @@ -59,6 +59,15 @@ object TaskMessages {

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46172807 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java --- @@ -402,21 +405,53 @@ public void onComplete(Throwable

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46174475 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/messages/JobManagerMessages.scala --- @@ -95,6 +95,14 @@ object JobManagerMessages {

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46159742 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46161133 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -569,6 +571,69 @@ public int compare(JobStatusMessage o1,

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread rmetzger
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46160278 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46160961 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -569,6 +571,69 @@ public int compare(JobStatusMessage o1,

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46162472 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendStopTest.java --- @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46162322 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendStopTest.java --- @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r46160903 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-27 Thread tillrohrmann
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-160100311 I'd volunteer as a shepherd of this PR. @mjsax I'll review your PR at the latest next week. Sorry for the long period of inactivity from my side. --- If

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45640243 --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.jade --- @@ -43,6 +43,10 @@ nav.navbar.navbar-default.navbar-fixed-top.navbar-main(ng-if="job")

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45640613 --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.jade --- @@ -43,6 +43,10 @@

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45639014 --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.jade --- @@ -43,6 +43,10 @@

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45639008 --- Diff: flink-runtime-web/web-dashboard/web/partials/jobs/job.html --- @@ -32,7 +32,7 @@ - {{ job['end-time'] |

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-158912414 Just rebased. Can you please review @StephanEwen @tillrohrmann --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-158913013 @sachingoel0101 Is the UI change ok? --- 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-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-158929824 I just investigated the failing test. The test is new and passed up to know. @uce changed `ExecutionGraph.cancel()` recently, allowing to cancel a job when it is in state

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45634054 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java --- @@ -188,9 +189,12 @@ public

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45635435 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java --- @@ -188,9 +189,12 @@ public WebRuntimeMonitor(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45635941 --- Diff: flink-runtime-web/web-dashboard/web/partials/jobs/job.html --- @@ -32,7 +32,7 @@ - {{ job['end-time'] |

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45636371 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java --- @@ -188,9 +189,12 @@ public

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45634227 --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.jade --- @@ -43,6 +43,10 @@

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread sachingoel0101
Github user sachingoel0101 commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45634353 --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.svc.coffee --- @@ -216,4 +216,7 @@ angular.module('flinkApp') # proper

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45635078 --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.jade --- @@ -43,6 +43,10 @@ nav.navbar.navbar-default.navbar-fixed-top.navbar-main(ng-if="job")

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-23 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r45635645 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/WebRuntimeMonitor.java --- @@ -188,9 +189,12 @@ public WebRuntimeMonitor(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-19 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-158050729 Opened a new JIRA for the failing build. Seems to be a instable test. (Build passed on my personla travis: https://travis-ci.org/mjsax/flink/builds/91941028) Please review

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread sachingoel0101
Github user sachingoel0101 commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157817216 Hi @mjsax , your change to coffeescript aren't ported to the actual javascript. It appears you forgot to run `gulp`. --- If your project is set up for it, you

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157832022 Thanks. Great. It works now. I was not aware the I have to build the UI manually. Would it be possible to integrate it into maven build? At least we should provide a

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157833952 There is a readme: https://github.com/apache/flink/tree/master/flink-runtime-web --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157834943 How could I miss this...?? Sorry. My own fault... --- 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

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157712835 @sachingoel0101 I have a problem testing my changes to the WebUI. It seems that the browser does not load the correct (ie, changed) scripts... Check out the screenshot --

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-18 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157901595 Stop in WebUI is now working. After rebase, I realized that cancel was changed to GET for yarn. Do we need to implement stop the same way for yarn @mxm ? --- If your

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-17 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157442077 I just rebased this. Due to the new JobManager WebUI, I added a second commit to add STOP button to WebUI (which this does not work yet). I will squash both commits later

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-17 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157521508 I am confused why the build failed... It worked locally **and** on my personal travis: https://travis-ci.org/mjsax/flink/builds/91638981 Furthermore, if I

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-11-17 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-157529908 Travis is caching the contents of the .m2 directory. Maybe that caused some weird issues. I also had some travis issues which let tests fail with some very

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-10-20 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-149580271 Is there any news here? --- 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

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-27 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-143539717 Done. I split the PR and squashed the commit. @StephanEwen please let me know if it is ok. --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r40334026 --- Diff: flink-clients/src/main/java/org/apache/flink/client/program/Client.java --- @@ -452,6 +453,73 @@ public void cancel(JobID jobId) throws

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r40333992 --- Diff: flink-clients/src/main/java/org/apache/flink/client/program/Client.java --- @@ -452,6 +453,73 @@ public void cancel(JobID jobId) throws

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142970422 There are some crucial issues still with this pull request: Most importantly, it changes parts that it needs not change, altering system behavior. That

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142971028 What would really help to review this is a summary of how it actually works. We currently have to piece that together from the code, which takes quite some time...

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142974060 There are also many whitespace changes in this PR, probably due to the IDE settings, which trim lines in all files that were opened... --- If your project is set up

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r40333103 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -588,15 +653,16 @@ protected int cancel(String[] args) {

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread tillrohrmann
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142967362 I've just looked into the implementation of `Task.cancelTask` and there we also spawn a new thread to cancel the task. Thus, the semantics is the same for the

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r40331106 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -263,7 +268,7 @@ protected int run(String[] args) {

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r40333050 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -534,7 +536,70 @@ public int compare(JobStatusMessage o1,

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142973877 I am curious here, is the `stop()` method implemented differently in any function differently than the `cancel()` method? What would speak against

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-24 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142999802 Thanks for the review! Just to clarify: I did not apply any changes "just for fun". But I agree that this PR contains more changes as are actually related to this PR. For

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-22 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142345277 Thanks. Makes sense. I wait for the feedback of Till and Stephan and fix this afterwards. This PR need a rebase anyway, that I can do all of this stuff when merging (if

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-22 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142357249 Will have a look at this tomorrow. This touches some critical parts, so it needs a bit of time and calm to review. --- If your project is set up for it, you can

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-22 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-142331659 I think the implementation of `stopExecution()` is fine. You are executing the call in a separate thread and you are logging exceptions. A blocking stop() method

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-21 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-141926593 Any news here? What is the opinion on Till's concern about using a `Future`? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-15 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-140301180 Hi, I cleand-up all old commits, and put a new commit on top introducing `SourceFunction.stop()` and unblock stop signal using an own thread. Please give feedback.

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-15 Thread tillrohrmann
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-140335366 IMO, it would be better to wrap the `stopExecution` call in the `TaskManager` into a `Future`. This has the following reasons: First of all, with the current

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-15 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-140375020 I thinks it should be fine. The `TaskOperationResult` should only indicate, that the signal was delivered successful (ie, only sent to "streaming sources"). All other

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-10 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-139194317 > The cancel only sets an internal variable. That is probably true for all sources that Flink provides. But the interface for sources is a public API, and

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-08 Thread aljoscha
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-138569872 The `cancel` in the SourceFunction interface? As I said, I think it should be non-blocking. The cancel only sets an internal variable. The source must then respond by

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-08 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-138587744 Yes. That is what I assumed writing the code for this PR. The question is, if you still simply assume that `cancel` behaves this way or try to enforce it somehow.

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-08 Thread aljoscha
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-138651481 I would be in favor of documenting it in the JavaDoc. What do you think @rmetzger as the one who probably knows the Kafka Source best. --- If your project is set up

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-07 Thread rmetzger
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38856545 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-07 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38859306 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-07 Thread mjsax
Github user mjsax commented on the pull request: https://github.com/apache/flink/pull/750#issuecomment-138278500 Any input on the blocking/non-blocking question about `cancel` ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-07 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38863884 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-07 Thread rmetzger
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38863585 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-03 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38635830 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread aljoscha
Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38508028 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread aljoscha
Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38507948 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread rmetzger
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38511039 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread rmetzger
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38511106 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38515501 --- Diff: docs/apis/cli.md --- @@ -185,6 +189,18 @@ Action "list" lists running and scheduled programs. -s,--scheduledShow only

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-02 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38516042 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add "stop" signal to cleanly shut...

2015-09-01 Thread mjsax
Github user mjsax commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38458556 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-30 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38270665 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-28 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38179569 --- Diff:

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-28 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38179508 --- Diff:

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-28 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38186444 --- Diff: flink-clients/src/test/java/org/apache/flink/client/CliFrontendInfoTest.java --- @@ -107,28 +89,28 @@ public InfoTestCliFrontend(int

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-28 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38179870 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -411,6 +411,23 @@ class TaskManager(

[GitHub] flink pull request: [FLINK-2111] Add stop signal to cleanly shut...

2015-08-28 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/750#discussion_r38186259 --- Diff: flink-clients/src/main/java/org/apache/flink/client/CliFrontend.java --- @@ -588,15 +653,16 @@ protected int cancel(String[] args) {

<    1   2   3   >