[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-03-05 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1150#issuecomment-192798074 @revans2addressed --- 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

[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-03-04 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1150#issuecomment-192233525 @revans2 can you have a 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

[GitHub] storm pull request: STORM-1492 With nimbus.seeds set to default, a...

2016-02-25 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1082#issuecomment-189140525 I find that when we set `nimbus.seeds` as ip, such as `nimbus.seeds: [10.0.0.10]`. Then a nimbus for 10.0.0.10 will be 'Offline'. That's bec

[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-02-25 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1150#issuecomment-189127326 @abhishekagarwal87 Thanks for your hint. I have moved `test-clojure-serialization` back to *serialization-test.clj*, and squashed the commits. --- If your

[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-02-25 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1150#discussion_r54198829 --- Diff: storm-core/test/jvm/org/apache/storm/serialization/SerializationTest.java --- @@ -0,0 +1,160 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: [STORM-1576] fix ConcurrentModificationExcepti...

2016-02-25 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1151#issuecomment-189077703 +1 looks good to me --- 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

[GitHub] storm pull request: Documentation for cgroup support in Storm

2016-02-25 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1054#discussion_r54195364 --- Diff: documentation/cgroups_in_storm.md --- @@ -0,0 +1,65 @@ +# CGroups in Storm + +CGroups are used by Storm to limit the resource usage of

[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-02-25 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1150#discussion_r54081671 --- Diff: storm-core/test/jvm/org/apache/storm/serialization/SerializationTest.java --- @@ -0,0 +1,160 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: [STORM-1250]: port backtype.storm.serializatio...

2016-02-24 Thread wuchong
GitHub user wuchong opened a pull request: https://github.com/apache/storm/pull/1150 [STORM-1250]: port backtype.storm.serialization-test to java 1. Move `validate-kryo-conf-basic` and `validate-kryo-conf-fail` to `TestConfigValidate.java` 2. I'm not sure wether to tran

[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...

2016-02-24 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1149#issuecomment-188582664 +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

[GitHub] storm pull request: [STORM-1574] Better handle backpressure thread...

2016-02-24 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1149#discussion_r54043672 --- Diff: storm-core/test/jvm/org/apache/storm/utils/WorkerBackpressureThreadTest.java --- @@ -0,0 +1,68 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Documentation for cgroup support in Storm

2016-02-24 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1054#discussion_r54042400 --- Diff: documentation/cgroups_in_storm.md --- @@ -0,0 +1,65 @@ +# CGroups in Storm + +CGroups are used by Storm to limit the resource usage of

[GitHub] storm pull request: [STORM-1254]: port ui.helper to java

2016-02-24 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1139#issuecomment-188546255 @revans2 It's a bug which has been fixed in #1119 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as wel

[GitHub] storm pull request: [STORM-1254]: port ui.helper to java

2016-02-24 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1139#issuecomment-188174878 @revans2 @abhishekagarwal87 @hustfxj Thanks for your review, I have addressed all the comments. Can you have a look again? --- If your project is set up

[GitHub] storm pull request: [STORM-1254]: port ui.helper to java

2016-02-24 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1139#discussion_r53916420 --- Diff: storm-core/src/clj/org/apache/storm/ui/helpers.clj --- @@ -46,197 +46,3 @@ (fn [req] --- End diff -- I prefer to leave it here

[GitHub] storm pull request: STORM-1569: Adding option in nimbus to specify...

2016-02-23 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1144#issuecomment-188100464 +1 LGTM --- 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-1572] throw NPE when parsing the comman...

2016-02-23 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1142#issuecomment-188100054 +1 except a minor comment --- 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

[GitHub] storm pull request: [STORM-1572] throw NPE when parsing the comman...

2016-02-23 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1142#discussion_r53898826 --- Diff: storm-core/src/jvm/org/apache/storm/command/CLI.java --- @@ -238,10 +238,13 @@ public CLIBuilder arg(String name, Parse parse, Assoc assoc

[GitHub] storm pull request: [STORM-1254]: port ui.helper to java

2016-02-22 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1139#issuecomment-187520534 @abhishekagarwal87 @hustfxj Thanks for your suggestion, I have addressed all your comments. And fix the commits. Let me know if you find anything else

[GitHub] storm pull request: [STORM-1254]: port ui.helper to java

2016-02-22 Thread wuchong
GitHub user wuchong opened a pull request: https://github.com/apache/storm/pull/1139 [STORM-1254]: port ui.helper to java port ui.helper to java 1. I find that `defelem table` is never be called, so I removed it. 2. I haven't translated `defn requests-middleware

[GitHub] storm pull request: [STORM-1558] Utils in java breaks component pa...

2016-02-22 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/1119#issuecomment-187101294 I find that `event-log-link` breaks component page due to the same reason. https://github.com/apache/storm/blob/master/storm-core/src/clj/org/apache/storm/ui

[GitHub] storm pull request: [STORM-1556]nimbus.clj/wait-for-desired-code-r...

2016-02-18 Thread wuchong
GitHub user wuchong opened a pull request: https://github.com/apache/storm/pull/1121 [STORM-1556]nimbus.clj/wait-for-desired-code-replication wrong reset for current-replication-count-jar in local mode We do not go to count the number of jar-replication in local mode, but will

[GitHub] storm pull request: [STORM-1257] port backtype.storm.zookeeper to ...

2016-01-26 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1047#discussion_r50827653 --- Diff: storm-core/src/jvm/org/apache/storm/zookeeper/Zookeeper.java --- @@ -0,0 +1,355 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] storm pull request: [STORM-1257] port backtype.storm.zookeeper to ...

2016-01-26 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/1047#discussion_r50827587 --- Diff: storm-core/src/jvm/org/apache/storm/zookeeper/LeaderElectorImp.java --- @@ -0,0 +1,135 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm 1379 Removed Redundant Structure

2015-12-28 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/932#issuecomment-167676628 The commits is unimportant . That's fine to merge It. 发自我的 iPhone > 在 2015年12月29日,04:49,Derek Dagit

[GitHub] storm pull request: [STORM-1387] workers-artifacts directory confi...

2015-12-13 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/942#issuecomment-164321832 +1 LGTM --- 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 1379 Removed Redundant Structure

2015-12-09 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/932#issuecomment-163461026 +1 Could you rebase your commits into one? the last two commits change only one/two lines. --- If your project is set up for it, you can reply to this email and

[GitHub] storm pull request: Use File.separator for OS independence in Blob...

2015-12-07 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/928#issuecomment-162734475 +1 nice catch --- 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-1040. SQL support for Storm

2015-12-06 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/919#issuecomment-162390949 nice work! +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

[GitHub] storm pull request: [STORM-1364] logs the storm version when start...

2015-12-03 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/916#issuecomment-161851976 +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

[GitHub] storm pull request: [STORM-1210] Set Output Stream id in KafkaSpou...

2015-12-02 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/885#issuecomment-161507454 +1 Nice work. It is much clearer now. --- 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

[GitHub] storm pull request: [STORM-1210] Set Output Stream id in KafkaSpou...

2015-12-02 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/885#discussion_r46510092 --- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java --- @@ -27,8 +27,7 @@ public String zkRoot = null; public String id

[GitHub] storm pull request: STORM-1353 Update "jstorm-import" branch to th...

2015-12-02 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/904#issuecomment-161286041 Hi @ptgoetz , thanks for the note. We have tried a lot of alternative solutions, such as: d3, chart.js, xCharts, chartist.js and ECharts. Finally we find that [ECharts

[GitHub] storm pull request: STORM-1353 Update "jstorm-import" branch to th...

2015-11-25 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/904#issuecomment-159601051 +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

[GitHub] storm pull request: STORM-1009: link on http://storm.apache.org/do...

2015-11-19 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/882#issuecomment-158260200 LGTM. +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

[GitHub] storm pull request: add storm-id to worker log filename

2015-11-17 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/886#issuecomment-157597586 @jessicasco yes, you are right. The `workers-artifacts` feature is merged by @zhuoliu , you may ask him. --- If your project is set up for it, you can reply to

[GitHub] storm pull request: add storm-id to worker log filename

2015-11-17 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/886#issuecomment-157588970 We have topology-id and port in log directory, current log structures under storm-local/workers-artifacts: ``` --topoId0 --topoId1 --port1

[GitHub] storm pull request: [STORM-1198] Web UI to show resource usages an...

2015-11-17 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/875#discussion_r45153774 --- Diff: storm-core/src/ui/public/index.html --- @@ -143,6 +143,12 @@ ] }); $('#topology-summary [data-t

[GitHub] storm pull request: [STORM-1208] Guard against NPE, and avoid usin...

2015-11-17 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/881#issuecomment-157572950 +1 LGTM --- 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-1210] Set Output Stream id in KafkaSpou...

2015-11-17 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/885#issuecomment-157342872 Looks good to me. But the condition code (`topicAsStreamId` and `outputStreamId`) is a little reduplicate as the logic is almost the same. --- If your project

[GitHub] storm pull request: [STORM-1210] Set Output Stream id in KafkaSpou...

2015-11-17 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/885#issuecomment-157341386 Looks good to me. But the condition code is a little reduplicate and verbose , I prefer to check only one variable to set streamId with reseting outputStreamId

[GitHub] storm pull request: [STORM-1208] Guard against NPE, and avoid usin...

2015-11-16 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/881#discussion_r45027342 --- Diff: storm-core/src/jvm/backtype/storm/metric/internal/LatencyStatAndMetric.java --- @@ -242,7 +248,7 @@ private synchronized void rotate(long lat, long

[GitHub] storm pull request: STORM-1009: link on http://storm.apache.org/do...

2015-11-16 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/882#discussion_r45026214 --- Diff: documentation/Kestrel-and-Storm.md --- @@ -7,7 +7,7 @@ This page explains how to use to Storm to consume items from a Kestrel cluster

[GitHub] storm pull request: STORM-1009:Link on http://storm.apache.org/doc...

2015-11-16 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/880#issuecomment-157281699 Maybe we should close this PR as the new same pull request #882 have been created. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: Added in feature diff

2015-11-12 Thread wuchong
Github user wuchong commented on a diff in the pull request: https://github.com/apache/storm/pull/877#discussion_r44742223 --- Diff: STORM_JSTORM_FEATURE_DIFF.md --- @@ -0,0 +1,30 @@ + feature | Storm-0.11.0-SNAPSHOT (and expected pull requests) | JStorm-2.1.0 | Notes | JIRA

[GitHub] storm pull request: STORM-1129: Use topology name instead of id in...

2015-11-06 Thread wuchong
Github user wuchong commented on the pull request: https://github.com/apache/storm/pull/854#issuecomment-154403920 +1 In Alibaba, we implement our monitor system using topology name instead of id. And it is very useful , as users often resubmit topology several times , and