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

[GitHub] storm pull request: STORM-1145: Have IConnection push tuples inste...

GitHub user revans2 opened a pull request:

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

     STORM-1145: Have IConnection push tuples instead of pull them.

    This pull request is based off of #765 because it moved the overflow buffer into the disruptor queue itself, and we don't want the netty thread to block when doing routing.
    
    This does not improve the maximum throughput much, but because it removes an extra thread and an extra queue that a tuple must go through when being sent to another worker the 99%-ile, 99.9%-ile and worst case latency goes down some.

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

    $ git pull https://github.com/revans2/incubator-storm server-callback

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

    https://github.com/apache/storm/pull/830.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 #830
    
----
commit c415d321cc7cb5e5a7bf91ab9a309e9504896fbb
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date:   2015-09-22T20:37:25Z

    Added batching of tuples at a disruptor queue level.

commit a7165d7f5a291502aa0fa4fd77ac0b19dcc3eca7
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date:   2015-10-28T21:18:08Z

    STORM-1145: Have IConnection push tuples instead of pull them.
    Move deserialization to the IConnectionCallback so only Tuple batches are flowing through the system, not unserialized byte arrays.

----


---
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-1145: Have IConnection push tuples inste...

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

    https://github.com/apache/storm/pull/830#discussion_r43574714
  
    --- Diff: storm-core/src/clj/backtype/storm/disruptor.clj ---
    @@ -27,10 +27,10 @@
        :single-threaded ProducerType/SINGLE})
     
     (defnk disruptor-queue
    -  [^String queue-name buffer-size timeout :producer-type :multi-threaded]
    +  [^String queue-name buffer-size timeout :producer-type :multi-threaded :batch-size 100 :batch-timeout 1]
    --- End diff --
    
    can we make these default batch-size values come from config


---
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-1145: Have IConnection push tuples inste...

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

    https://github.com/apache/storm/pull/830#issuecomment-154525434
  
    I just rebased on the latest code in master, so not the only diffs you see should be what this patch is doing.  Before it was based off of other code that was really a part of other diffs.
    
    Please take a look again.


---
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-1145: Have IConnection push tuples inste...

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

    https://github.com/apache/storm/pull/830#issuecomment-155104855
  
    +1


---
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-1145: Have IConnection push tuples inste...

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

    https://github.com/apache/storm/pull/830#issuecomment-156174575
  
    @kishorvpatil I just rebased, please take another quick look.


---
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-1145: Have IConnection push tuples inste...

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

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


---
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.
---