[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2016-03-07 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/296 --- 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 enabl

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2016-03-07 Thread knusbaum
Github user knusbaum commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-193485875 Closing this. Feel free to reopen with updates. --- 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 p

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-10-14 Thread knusbaum
Github user knusbaum commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r42055642 --- Diff: storm-core/src/clj/backtype/storm/daemon/supervisor.clj --- @@ -155,7 +155,9 @@ (or (not (contains? approved-ids id))

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-10-14 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-148202852 The code still looks good. It needs to be upmerged because the heartbeats have moved to use thrift. I would love to see this go in. --- If your project is set up for i

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-10-14 Thread knusbaum
Github user knusbaum commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-148184473 @caofangkun, @revans2, @kishorvpatil, @xiaokang Any movement on this? --- If your project is set up for it, you can reply to this email and have your reply appe

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-17 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r26581260 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -392,6 +392,15 @@ (.addArgument command a)) (.execute (DefaultExecutor.) command

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-12 Thread caofangkun
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r26362192 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -392,6 +392,15 @@ (.addArgument command a)) (.execute (DefaultExecutor.) comm

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-11 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-78341357 For the most part it seems fine. My manual attempts to kill workers all are foiled very quickly, but I would like to not have the supervisor be so noisy with the ps comma

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-11 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r26240829 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -392,6 +392,15 @@ (.addArgument command a)) (.execute (DefaultExecutor.) command

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-04 Thread caofangkun
Github user caofangkun commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-77296982 @kishorvpatil I have resolved merge conflicts , please review the PR again. Thank you . --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-02 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-76745378 @caofangkun, could you please upmerge this change? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-72706534 The code change looks OK, but I am seeing test failures in supervisor_test.clj. I also would prefer to have us cache the Process that we used to launch the external proce

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-01-26 Thread caofangkun
Github user caofangkun commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-71582734 UT Tests passed and Ready for reivew. --- 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 pro

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-11-19 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r20565435 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " nam

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-11-18 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r20535744 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " name)))

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-11-18 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r20528140 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " nam

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-11-17 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r20450306 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " name)))

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-25 Thread caofangkun
Github user caofangkun closed the pull request at: https://github.com/apache/storm/pull/293 --- 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 e

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/293#issuecomment-60419007 @caofangkun If this pull request has been superseded by #296 would you mind closing this? --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-10-21 Thread xiaokang
Github user xiaokang commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-59948431 Supervisor and workers needed to be restarted if we change the WorkerHeartbeat data structure by adding a process-id field. It may be more easy to read worker and its chi

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-17 Thread itaifrenkel
Github user itaifrenkel commented on the pull request: https://github.com/apache/storm/pull/293#issuecomment-59499800 Hi @caofangkun. Could you then close this pull request? Please see my comments in #296 --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-10-17 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r19012098 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " nam

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-10-17 Thread caofangkun
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r19007702 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " name

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-10-16 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/296#discussion_r19003251 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -372,6 +372,13 @@ (throw (RuntimeException. (str "Got unexpected process name: " nam

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread caofangkun
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/293#discussion_r18949050 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -356,6 +356,10 @@ (first split) )) + (defn exists-process? --- En

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/293#discussion_r18947137 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -356,6 +356,10 @@ (first split) )) + (defn exists-process? --- E

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread caofangkun
Github user caofangkun commented on the pull request: https://github.com/apache/storm/pull/293#issuecomment-59339694 @itaifrenkel Thank you for your comments I have fixed with your suggestion, in https://github.com/apache/storm/pull/296 --- If your project is set up for i

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2014-10-16 Thread caofangkun
GitHub user caofangkun opened a pull request: https://github.com/apache/storm/pull/296 STORM-532:Supervisor should restart worker immediately, if the worker pr... ...ocess does not exist any more You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/293#discussion_r18945173 --- Diff: storm-core/src/jvm/backtype/storm/spout/SleepSpoutWaitStrategy.java --- @@ -33,7 +33,7 @@ public void prepare(Map conf) { @Override

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/293#discussion_r18944932 --- Diff: storm-core/src/clj/backtype/storm/daemon/supervisor.clj --- @@ -114,6 +114,8 @@ :disallowed

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-16 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/293#discussion_r18943315 --- Diff: storm-core/src/clj/backtype/storm/util.clj --- @@ -356,6 +356,10 @@ (first split) )) + (defn exists-process? --- E

[GitHub] storm pull request: STORM-532,Supervisor should restart worker imm...

2014-10-14 Thread caofangkun
GitHub user caofangkun opened a pull request: https://github.com/apache/storm/pull/293 STORM-532,Supervisor should restart worker immediately, if the worker pr... https://issues.apache.org/jira/browse/STORM-532 For now if the worker process does not exist any more