[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @harshach all of those tests look good to me. I just want to be sure that we do them in both a multi-worker and multi-node setup. --- If your project is set up for it, you can reply to this email a

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 @roshannaik The output collector is wrapped separately by each bolt so it will have no effect. https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/topology/Basi

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2241 Also I wanted to add that I found another bug. The process latency metric is always right about a -1ms. Not sure what would be causing this. --- If your project is set up for it, you can reply to

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-08-11 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2270 Did some bad thing on the commits... Fixing it --- 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 featu

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-08-11 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2270 Last commit changes: 1. change iterator based loop to index based loop for better performance 2. add locality_aware example in ConstSpoutNullBoltTopo 3. only store necessary data in taskNe

[GitHub] storm issue #2268: STORM-2689: Simplify dependency configuration for storm-k...

2017-08-11 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2268 @HeartSaVioR You're right, it's nicer if the example is not relying on storm-kafka. I copied the missing classes to storm-kafka-client-examples, and added a bunch of documentation. --- If your projec

[GitHub] storm issue #2268: STORM-2689: Simplify dependency configuration for storm-k...

2017-08-11 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2268 We could get rid of the storm-starter dependency if we removed the DRPC demonstration from the Trident examples and replaced the RandomSentenceSpout. There's already a DRPC demo in storm-starter, so I d

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746643 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter; +

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746674 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter; +

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746702 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter; --

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746791 --- Diff: conf/defaults.yaml --- @@ -293,3 +293,28 @@ storm.daemon.metrics.reporter.plugins: # configuration of cluster metrics consumer storm

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746888 --- Diff: external/storm-autocreds/pom.xml --- @@ -15,9 +15,7 @@ See the License for the specific language governing permissions and limitations u

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132746948 --- Diff: storm-core/pom.xml --- @@ -700,10 +707,10 @@ org.eclipse.jetty org.apache.storm.shade

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132747934 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -139,6 +139,9 @@ @isString public static final String STORM_META_SERIALIZAT

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-08-11 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r132748013 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/DisruptorMetrics.java --- @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] storm issue #2241: STORM-2306 : Messaging subsystem redesign.

2017-08-11 Thread roshannaik
Github user roshannaik commented on the issue: https://github.com/apache/storm/pull/2241 @revans2 - I suspect we are talking about diff things wrt Thread safety in emit path and consequently I am missing what you are trying to communicate. There is no intention to require any syn