You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by errordaiwa <gi...@git.apache.org> on 2015/07/14 03:01:37 UTC

[GitHub] storm pull request: [STORM-935] Update Disruptor queue version to ...

GitHub user errordaiwa opened a pull request:

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

    [STORM-935] Update Disruptor queue version to 2.10.4

    Update version of Disruptor queue to 2.10.4, consumer of queue will use waitfor method without timeout. Test on storm 0.9.3, works fine.

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

    $ git pull https://github.com/errordaiwa/storm STORM-935

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

    https://github.com/apache/storm/pull/630.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 #630
    
----
commit f3e973fa4df7af68317e4e239c3c783756de1e75
Author: errordaiwa <xi...@outlook.com>
Date:   2015-07-13T09:46:16Z

    update version of Disruptor queue to 2.10.4, consumer of queue will use waitfor method without timeout

----


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121793879
  
    done.
    
    @HeartSaVioR @revans2 Thanks for validation and confirming.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-122131324
  
    I merged this into 0.10.x-branch and 0.9.x-branch by cherry-picked.
    
    Since there're different available sets of config validators among branches - master, 0.10.x, 0.9.x, I take the best proper validator to each branch.
    
    * master: NotNullPosIntegerValidator
    * 0.10.x: PositiveIntegerValidator
    * 0.9.x: IntegerValidator


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121140113
  
    I do a performance test using storm 0.9.3 with disruptor queue 2.10.4. The target topology is metioned in [STORM-503](https://github.com/apache/storm/pull/625). To make result more clear, I raise bolt num to 1000.
    
    Here is the CPU usage.
    + base (Storm running with nothing)
      + user: 2%
      + sys: 1.5%
    + no timeout
      + user: 4%
      + sys: 1.5%
    + 1000ms timeout
      + user: 5%
      + sys: 2%
    + 100ms timeout
      + user: 6%
      + sys: 4.5%
    + 10ms timeout
      + user: 17%
      + sys: 26%



---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121764919
  
    @revans2 Thanks for confirming. :)
    
    During running performance test I can't see any failed tuples from the UI in 10 minutes, so I think it is safe to merge.
    LGTM, +1.
    
    Btw, it would be better for users to change default timeout in order to let users not suffering STORM-503 with default setting.
    
    @errordaiwa Could you change default timeout to 1000ms?
    
    After changing default timeout I'll 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 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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-122124259
  
    It is merged to master, and I'm working on backport to 0.10.x and 0.9.x.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121150967
  
    @errordaiwa @amontalenti 
    1000ms timeout makes sense to me. 
    Actually 100ms timeout also makes sense to me, but I'd like to know our opinions about load aware.
    
    We're having a option to set timeout now, so it would be no issue.
    I'll run some performance tests and check no tuples are failed.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121128494
  
    @errordaiwa @arunmahadevan 
    I'm aware of unknown bug in Disruptor Queue, so I'd like to specify timeout value though some race conditions are fixed.
    We can have an experiment with adjusting timeout from 100 to 1000 and look at CPU usage.
    
    @errordaiwa 
    If you don't mind, I'd like to ask you a favor to experiment such things, since I'm not ready to experiment.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-122514207
  
    Great!
    
    @HeartSaVioR Sincerely thank you  for working on this!


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-122119993
  
    Since many users are already affected this situation, it should be included to 0.9.x line.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121749743
  
    I am fine with whichever timeout we pick. I am +1 for merging this in pending the results of the tests @HeartSaVioR is running.


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-121126833
  
    +1 @errordaiwa how is the cpu usage with high bolt count when idle ?


---
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-935] Update Disruptor queue version to ...

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

    https://github.com/apache/storm/pull/630#issuecomment-122113950
  
    +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-935] Update Disruptor queue version to ...

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

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


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