[GitHub] storm issue #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-30 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 Tests are done to compare following two code bases: - baseline: current head of 1.x-branch (62476f5237a72938c1f0d3fe65dbcdc446a2bd12) - netty 4.1.x: 1.x-branch-netty4

[GitHub] storm issue #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-30 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 @HeartSaVioR I rebased to 1.x-branch and also squashed the commits. Will attached the performance test results soon. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-29 Thread hsun-cnnxty
GitHub user hsun-cnnxty reopened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-29 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/1591 --- 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

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-24 Thread hsun-cnnxty
GitHub user hsun-cnnxty reopened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-24 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/1591 --- 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

[GitHub] storm issue #728: [STORM-1038] Upgraded netty to 4.x

2016-09-07 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/728 @kishorvpatil that's an interesting idea. You mean a feature flag to toggle between 3.x and 4.x? I will investigate the possibility. Btw, I have moved the work to #1591. -thanks

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-07 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/1591 --- 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

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-07 Thread hsun-cnnxty
GitHub user hsun-cnnxty reopened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-09-07 Thread hsun-cnnxty
Github user hsun-cnnxty commented on a diff in the pull request: https://github.com/apache/storm/pull/1591#discussion_r77873290 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -73,6 +73,20 @@ public static final String STORM_MESSAGING_NETTY_BUFFER_SIZE

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-08-18 Thread hsun-cnnxty
GitHub user hsun-cnnxty reopened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-08-18 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/1591 --- 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

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-08-12 Thread hsun-cnnxty
GitHub user hsun-cnnxty reopened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-08-12 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/1591 --- 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

[GitHub] storm issue #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-08-03 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 @satishd nett 4.x has removed auto-flush in write() and it requires user to explicitly call flush() or writeAndFlush(), so we need to flush for the last message in the logic flow. Fortunately

[GitHub] storm issue #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-07-30 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 Seems the Travis CI build will fail at random places which are not related to this PR. For example the last build failed due to `[ERROR] Failed to execute goal

[GitHub] storm issue #728: [STORM-1038] Upgraded netty to 4.x

2016-07-30 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/728 @HeartSaVioR @harshach I posted performance test results on #1591. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] storm issue #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-07-30 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 Performance test results are attached here. [perf_compare_netty_3vs4_1.x-branch.zip](https://github.com/apache/storm/files/392165/perf_compare_netty_3vs4_1.x-branch.zip) Baseline

[GitHub] storm issue #728: [STORM-1038] Upgraded netty to 4.x

2016-07-25 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/728 As this PR is for master, new PR #1591 is created for 1.x-branch. Performance tests to be done soon. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch

2016-07-25 Thread hsun-cnnxty
GitHub user hsun-cnnxty opened a pull request: https://github.com/apache/storm/pull/1591 STORM-1038: Upgrade netty to 4.x in 1.x-branch This is to add the feature to 1.x-branch. The original PR for master branch is #728. You can merge this pull request into a Git repository

[GitHub] storm issue #728: [STORM-1038] Upgraded netty to 4.x

2016-07-10 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/728 Sure. --- 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

[GitHub] storm issue #728: [STORM-1038] Upgraded netty to 4.x

2016-07-10 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/728 I am currently on vacation and will be back in two weeks. Will work on it as soon as I am back home. -thanks --- If your project is set up for it, you can reply to this email and have

[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

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2016-02-24 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-188634249 Just did another merge to keep it up to date with master branch. Not sure what the plan is now. I would be happy to make any changes that can help to make the final

[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-822 Implement Kafka 0.9 consumer API

2016-01-20 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-173285410 @revans2 and all, I feel my PR ([STORM-1015]: Allow Kafka offsets to be saved using Kafka's consumer offset management api) is kind of independent of this PR

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-01-19 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-173093649 I don't have much experience on how the collaborations be done in storm project. Happy to take any advices on what I can help. --- If your project is set up

[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-1015] Allow Kafka offsets to be saved u...

2016-01-06 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-169487103 @choang changes are made and latest code from master merged in. Please review whenever you have time. -thanks --- If your project is set up for it, you can reply

[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-1015] Allow Kafka offsets to be saved u...

2015-10-13 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-147937919 @choang In recent changes, I have made it possible to plug in custom store implementations. The custom implementation is given the opportunity to initialize itself

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-10-05 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-145586698 @choang: the refactored code is pushed @erikdw: I was referring to the internal Json structure used to store offsets as shown by example below

[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-1015] Allow Kafka offsets to be saved u...

2015-10-03 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-145317404 Hi Chi, Storm stores more than just offset/partition data in the "state", would it be necessary to declare? public interface

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-22 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-142364685 Erik, Pushed the change to rename config from "storm" to "zookeeper" (also fixed the typo). As of Kafka dependency, it is a good ques

[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

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-15 Thread hsun-cnnxty
Github user hsun-cnnxty commented on a diff in the pull request: https://github.com/apache/storm/pull/705#discussion_r39595284 --- Diff: external/storm-kafka/src/jvm/storm/kafka/KafkaDataStore.java --- @@ -0,0 +1,202 @@ +package storm.kafka; + +import

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-15 Thread hsun-cnnxty
Github user hsun-cnnxty commented on a diff in the pull request: https://github.com/apache/storm/pull/705#discussion_r39595289 --- Diff: external/storm-kafka/src/jvm/storm/kafka/PartitionStateManagerFactory.java --- @@ -0,0 +1,68 @@ +package storm.kafka; + +import

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-15 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-140633818 Erik, I added 2 reference links in the description with more information on "Kafka's consumer offset management api". As of migration fro

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-12 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-139828793 Fixed coding styles suggested by rmkellogg. --- 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

[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

[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

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-08 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-138791147 I assume you are referring to the json format of the spout "state" stored in ZK? The "state" will be saved using the exact same json format

[GitHub] storm pull request: [STORM-1015] Allow Kafka offsets to be saved u...

2015-09-08 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/705#issuecomment-138777397 Thanks for the advices. Will get them fixed as soon as I get some time. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: [Storm-650] Added new option to allow Kafka sp...

2015-08-27 Thread hsun-cnnxty
Github user hsun-cnnxty commented on the pull request: https://github.com/apache/storm/pull/703#issuecomment-135433954 Sure, will make the change and resubmit. Thanks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: [Storm-650] Added new option to allow Kafka sp...

2015-08-27 Thread hsun-cnnxty
Github user hsun-cnnxty closed the pull request at: https://github.com/apache/storm/pull/703 --- 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

[GitHub] storm pull request: Added new option to allow Kafka spout to save ...

2015-08-26 Thread hsun-cnnxty
GitHub user hsun-cnnxty opened a pull request: https://github.com/apache/storm/pull/703 Added new option to allow Kafka spout to save offset and other state using Kafka's offset management api Current Kafka spout stores the offsets (and some other states) inside ZK with its