You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2015/06/23 23:29:11 UTC

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

GitHub user HeartSaVioR opened a pull request:

    https://github.com/apache/storm/pull/602

    STORM-871 Change multilang heartbeat mechanism

    * Introduce new way of treating heartbeat
    ** subprocess has to update pid file's modified time periodically
    *** in default implementation we update pid file every 1 sec
    ** ShellSpout / ShellBolt checks pid file's modified time every 1 sec
    ** No more heartbeat tuple
    
    It may introduces language constraints, so we need to gather opinions from user/dev mailing list to make sure we can safely change mechanism.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/HeartSaVioR/storm STORM-871

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/602.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #602
    
----
commit 13e63d25e1f5ce6f50b3a527b8a9bceaf9116660
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2015-06-23T21:03:42Z

    STORM-871 Change multilang heartbeat mechanism
    
    * Introduce new way of treating heartbeat
    ** subprocess has to update pid file's modified time periodically
    *** in default implementation we update pid file every 1 sec
    ** ShellSpout / ShellBolt checks pid file's modified time every 1 sec
    ** No more heartbeat tuple

----


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/602#issuecomment-114656326
  
    Gathering opinions: http://mail-archives.apache.org/mod_mbox/storm-user/201506.mbox/%3CCAF5108iiB0_%2By_MzReVUF7LWDsQgyi9jFK1QNPAupYs6_xj-jw%40mail.gmail.com%3E


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/602#discussion_r34323522
  
    --- Diff: storm-multilang/python/src/main/resources/resources/storm.py ---
    @@ -29,6 +30,26 @@
     json_encode = lambda x: json.dumps(x)
     json_decode = lambda x: json.loads(x)
     
    +# scheduled timer running job at fixed rate (note: GIL!)
    --- End diff --
    
    Thanks @dan-blanchard ,
    I experimented about python's GIL just now, and with python 2.7.6 in OSX I found that other thread can hold CPU more than 1 sec when timer is expired at that time. 
    https://gist.github.com/HeartSaVioR/34d90cdd6af906e72935
    
    Actually I wasn't affected this issue during I was working with Python cause it was I/O intensive job, and seems like it isn't same to CPU intensive job.
    
    Default tick time is somewhat very long. I found one document which says tick time is about ~6.5 secs, which doesn't meet our requirement.
    
    GIL is a major problem, and maybe I should check it with Ruby.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/602#issuecomment-114660947
  
    We need to change document when we apply this and release.
    http://storm.apache.org/documentation/Multilang-protocol.html


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/602#issuecomment-121612188
  
    After discussed with @dan-blanchard , I gave up introducing new multilang heartbeat mechanism.
    It would be new requirement to storm multilang libraries, which library developers already struggled with current heartbeat mechanism.
    Above all, I can't think about overcoming GIL.
    
    I'll close this issue, but to stick with current mechanism STORM-742 should be merged.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR closed the pull request at:

    https://github.com/apache/storm/pull/602


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by dan-blanchard <gi...@git.apache.org>.
Github user dan-blanchard commented on a diff in the pull request:

    https://github.com/apache/storm/pull/602#discussion_r34284915
  
    --- Diff: storm-multilang/python/src/main/resources/resources/storm.py ---
    @@ -29,6 +30,26 @@
     json_encode = lambda x: json.dumps(x)
     json_decode = lambda x: json.loads(x)
     
    +# scheduled timer running job at fixed rate (note: GIL!)
    --- End diff --
    
    As you note, the GIL is going to be a major problem with this approach.  To the point, that this will not work as intended and you will still have timeout issues if processing an individual tuple takes too long.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-871 Change multilang heartbeat mechanism

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/602#discussion_r33101920
  
    --- Diff: storm-core/test/clj/backtype/storm/multilang_test.clj ---
    @@ -45,9 +45,9 @@
                       {"2" (thrift/mk-shell-bolt-spec {"1" :shuffle} [executor (str "tester_bolt." file-extension)] ["word"] :parallelism-hint 1)})]
              (submit-local-topology nimbus
                    "test"
    -               {TOPOLOGY-WORKERS 20 TOPOLOGY-MESSAGE-TIMEOUT-SECS 3 TOPOLOGY-DEBUG true}
    +               {TOPOLOGY-WORKERS 20 TOPOLOGY-MESSAGE-TIMEOUT-SECS 3 TOPOLOGY-DEBUG true SUPERVISOR-WORKER-TIMEOUT-SECS 3}
                    topology)
    -       (Thread/sleep 10000)
    +       (Thread/sleep 20000)
    --- End diff --
    
    Adjust option regarding subprocess heartbeat and make test longer for improving test more accurate.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---