[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-02-25 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-188822872 Yes I am aware of it.  We are in the process of merging with the JStorm project, and part of this merger involves moving most of the clojure code to java.  In all

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-02-24 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-188640434 @revans2 Just merged code from master and seems there is performance degradation with recent changes. I noticed that it not only affects this branch, but

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-20 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-173483216 With some refactoring, now it can sustain throughput of 20,000 /sec which it was not able to before. But latency at 20,000 /sec is still much higher than 3.x (5+

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-10 Thread darionyaphet
Github user darionyaphet commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-170354259 mark --- 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] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r49227209 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/KerberosSaslClientHandler.java --- @@ -46,56 +44,48 @@ public

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r49227072 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/ISaslClient.java --- @@ -17,11 +17,9 @@ */ package backtype.storm.messaging.netty;

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-170100773 Everything looks really good now. just a few minor nits. I still have not found time to run some performance tests, but I will try to do that today. --- If your

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-170145276 I just ran some performance tests using https://github.com/apache/storm/blob/master/examples/storm-starter/src/jvm/storm/starter/ThroughputVsLatency.java I ran

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-170147036 Cool, at least we get some numbers to compare. I will see if there is default setting need to be changed for netty 4. --- If your project is set up for it, you can

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread harshach
Github user harshach commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r49248827 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -17,41 +17,31 @@ */ package backtype.storm.messaging.netty;

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r49227426 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/NettyUncaughtExceptionHandler.java --- @@ -21,6 +21,24 @@ import org.slf4j.Logger;

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2016-01-08 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-170149708 @hsun-cnnxty I hope it is something like that. You should be able to run the tests yourself too. They are not that complex. I build ``` mvn clean install

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-12-14 Thread rfarivar
Github user rfarivar commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r47586860 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -182,7 +177,7 @@ private boolean connectionEstablished(Channel channel) {

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-12-14 Thread rfarivar
Github user rfarivar commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r47586629 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -23,24 +23,17 @@ import backtype.storm.metric.api.IStatefulObject;

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-12-14 Thread rfarivar
Github user rfarivar commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r47586615 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -23,24 +23,17 @@ import backtype.storm.metric.api.IStatefulObject;

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-12-14 Thread hsun-cnnxty
Github user hsun-cnnxty commented on a diff in the pull request: https://github.com/apache/storm/pull/728#discussion_r47597958 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -182,7 +177,7 @@ private boolean connectionEstablished(Channel channel) {

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-10-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-145253788 @hsun-cnnxty, The changes look good to me. Please upmerge to the latest code. Then I would like to run some performance tests on it to see how it compares to the

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-10-03 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-145318156 @revans2, just merged with the latest master. I don't have a decent storm cluster for performance test. With a small local cluster on single machine. I had tried

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-09-19 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-141744079 Upgraded to latest 4.0.31.Final and changed the buffer allocation to lett netty choose the best default based on the platform. --- If your project is set up for it,

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-09-09 Thread hsun-cnnxty
GitHub user hsun-cnnxty opened a pull request: https://github.com/apache/storm/pull/728 [STORM-1038] Upgraded netty to 4.x Upgraded the netty transportation layer to 4.x to take advantage of its memory management efficiency. You can merge this pull request into a Git repository by

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-09-09 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-139122346 @hsun-cnnxty Great work. Since it can affect performance I recommend you to include benchmark test result. You can refer #521 (comment) to how to do it.

[GitHub] storm pull request: [STORM-1038] Upgraded netty to 4.x

2015-09-09 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/728#issuecomment-139123790 Good suggestion. I will get to work on them as soon as I get some time. I am also curious to verify the memory efficiency claimed by 4.x --- If your project is set