[GitHub] storm issue #1674: STORM-2083: Blacklist scheduler

2016-09-27 Thread nilday
Github user nilday commented on the issue: https://github.com/apache/storm/pull/1674 Thanks for you all guys. I will consider seriously all your suggestions. But currently I'll be still working on 1.0.x branch ,since my company has just update to 1.0 version and master branch is

[GitHub] storm pull request #1674: STORM-2083: Blacklist scheduler

2016-09-27 Thread nilday
Github user nilday commented on a diff in the pull request: https://github.com/apache/storm/pull/1674#discussion_r80839269 --- Diff: storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java --- @@ -0,0 +1,212 @@ +package

[GitHub] storm pull request #1674: STORM-2083: Blacklist scheduler

2016-09-27 Thread nilday
Github user nilday commented on a diff in the pull request: https://github.com/apache/storm/pull/1674#discussion_r80839265 --- Diff: storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java --- @@ -0,0 +1,212 @@ +package

[GitHub] storm pull request #1674: STORM-2083: Blacklist scheduler

2016-09-27 Thread nilday
Github user nilday commented on a diff in the pull request: https://github.com/apache/storm/pull/1674#discussion_r80839227 --- Diff: storm-core/src/jvm/org/apache/storm/scheduler/blacklist/BlacklistScheduler.java --- @@ -0,0 +1,212 @@ +package

[GitHub] storm issue #1716: STORM-2126: fix NPE due to race condition in compute-new-...

2016-09-27 Thread abellina
Github user abellina commented on the issue: https://github.com/apache/storm/pull/1716 This can also be easily cherry-picked to 1.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

Re: [DISCUSS] Provision for dead-letter topic in storm

2016-09-27 Thread Ravi Sharma
Yes good idea, would love to have this functionality of passing some user defined data from bolt to spout on failures. Ravi On 27 Sep 2016 10:05 p.m., "Kyle Nusbaum" wrote: > It seems to me that this can be solved by allowing a user to attach some > arbitrary

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread wangli1426
Github user wangli1426 commented on the issue: https://github.com/apache/storm/pull/1703 Thanks for the prompt response. I will create a new pull request for master asap. Sent from my iPhone > On 28 Sep 2016, at 9:09 AM, Jungtaek Lim

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1703 Yes sure. You don't essentially need to upmerge/rebase old pull request if you want to provide new pull request based on current. --- If your project is set up for it, you can reply to this

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread wangli1426
Github user wangli1426 commented on the issue: https://github.com/apache/storm/pull/1703 Can I create a new PR based on the latest master version rather than upmerging this RP to master? The former would be much easier, since a lot of changes have been made from 1.x to 2.x.

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1703 @wangli1426 Sorry I haven't had time to review this right now. I'll take a look when I get time. Btw, since you're modifying ported clojure files, you need to port your change to

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80820718 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -926,10 +926,10 @@ public static final String UI_HTTPS_NEED_CLIENT_AUTH =

[GitHub] storm issue #1713: Storm 2124 show requested cpu mem for each component

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1713 I merged this into master. The test failures are unrelated. --- 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 #1713: Storm 2124 show requested cpu mem for each compone...

2016-09-27 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1713 --- 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

[GitHub] storm issue #1713: Storm 2124 show requested cpu mem for each component

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1713 +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

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80801551 --- Diff: storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java --- @@ -159,10 +162,6 @@ public void set_worker_hb(String path, byte[] data,

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread knusbaum
Github user knusbaum commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80798282 --- Diff: storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java --- @@ -159,10 +162,6 @@ public void set_worker_hb(String path, byte[]

[GitHub] storm issue #1717: Update eventhub client dependency. Move Storm-Eventhubs d...

2016-09-27 Thread raviperi
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1717 Below JIRA that reflects the need for the above work. STORM-2127 - Storm-eventhubs should use latest amqp and eventhubs-client versions --- If your project is set up for it, you can reply to

[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread raviperi
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 Created a new PR https://github.com/apache/storm/pull/1717 for the master branch. Created a new JIRA that reflects the need for the aboce work. STORM-2127 - Storm-eventhubs should use

[GitHub] storm issue #1713: Storm 2124 show requested cpu mem for each component

2016-09-27 Thread abellina
Github user abellina commented on the issue: https://github.com/apache/storm/pull/1713 @revans2 I added docs and filled in docs for topology and component endpoints. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] storm pull request #1717: Update eventhub client dependency. Move Storm-Even...

2016-09-27 Thread raviperi
GitHub user raviperi opened a pull request: https://github.com/apache/storm/pull/1717 Update eventhub client dependency. Move Storm-Eventhubs dependency ve… Update eventhub client dependency. Move Storm-Eventhubs dependency vesion definitions to parent pom. Introduce new schemes

Re: [DISCUSS] Provision for dead-letter topic in storm

2016-09-27 Thread Kyle Nusbaum
It seems to me that this can be solved by allowing a user to attach some arbitrary data to a call to fail(), which is passed to the spout. So there would be an override for fail in IOutputCollector which takes both the Tuple input and also some object to give to the spout. The spout's fail

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80791012 --- Diff: storm-core/src/jvm/org/apache/storm/cluster/PaceMakerStateStorage.java --- @@ -159,10 +162,6 @@ public void set_worker_hb(String path, byte[] data,

Re: Job exceeding 50 mins limit

2016-09-27 Thread Raghav Kumar Gautam
Great !!! CCing storm dev community. Thanks Anna. On Tue, Sep 27, 2016 at 12:06 PM, Anna Nagy wrote: > Perfect! Thanks again, Raghav. I just confirmed the custom timeout should > be all set - it's set at 90 minutes. Once the container-based infra timeout > is respected,

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80776662 --- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java --- @@ -152,7 +152,7 @@ public void open(Map config,

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80775363 --- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java --- @@ -152,7 +152,7 @@ public void open(Map config,

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80771322 --- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java --- @@ -152,7 +152,7 @@ public void open(Map config,

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80771003 --- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/StringEventDataScheme.java --- @@ -0,0 +1,69 @@

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80770524 --- Diff: external/storm-eventhubs/pom.xml --- @@ -1,113 +1,120 @@

[GitHub] storm pull request #1716: STORM-2126: fix NPE due to race condition in compu...

2016-09-27 Thread abellina
GitHub user abellina opened a pull request: https://github.com/apache/storm/pull/1716 STORM-2126: fix NPE due to race condition in compute-new-sched-assign… …ments/read-all-supervisor-details Work done by @d2r at Yahoo. You can merge this pull request into a Git

[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread raviperi
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 The latest PR addresses the above comments . Move dependencies to parent pom . Revert changes to versions to reflect naming convention. I will be sending a different PR to master

Re: [DISCUSS] Plan for merge SQE and Storm SQL

2016-09-27 Thread P. Taylor Goetz
Indeed it does not compile against any version due to the missing changes to storm-kafka and storm-redis that Morrigan pointed out. So for now I’m leaning toward a feature branch so we can at least import the code as is (knowing it will not compile). From there we can request pull requests

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread raviperi
Github user raviperi commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80753947 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT

[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread raviperi
Github user raviperi commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80753320 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT

Re: [DISCUSS] Provision for dead-letter topic in storm

2016-09-27 Thread Tech Id
Any more thoughts on this? Seems like a useful feature for all the spouts/bolts. On Wed, Sep 21, 2016 at 9:09 AM, S G wrote: > Thank you Aaron. > > We use Kafka and JMS spouts and several bolts - Elastic-Search, Solr, > Cassandra, Couchbase and HDFS in different

[GitHub] storm issue #1713: Storm 2124 show requested cpu mem for each component

2016-09-27 Thread jerrypeng
Github user jerrypeng commented on the issue: https://github.com/apache/storm/pull/1713 Did some manual testing with the UI and everything looks good +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

[GitHub] storm issue #1676: STORM-2085: Remove guava from storm-core pom.

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1676 No wait, dependency:analyze indicates that guava is used? Not sure how it compiled though. Investigating. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm issue #1676: STORM-2085: Remove guava from storm-core pom.

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1676 Nope I was wrong it compiles just fine and there is no guava except being pulled in by curator test. Yes +1 for this, and +1 for pulling it into 1.x if possible. --- If your project is set

[GitHub] storm issue #1676: STORM-2085: Remove guava from storm-core pom.

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1676 I'm +1 on this, but I am a little nervous I may have started using guava as part of the supervisor V2. Let me try and run some tests. --- If your project is set up for it, you can reply to this

[GitHub] storm issue #1713: Storm 2124 show requested cpu mem for each component

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1713 My tests look good I am +1 once the docs are updated. --- 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 issue #1692: Fix STORM-2017

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1692 +1 the change looks good. @kluoto is this an issue for master too? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1702 Is there a similar pull request to master? --- 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 #1702: Update Eventhub-Client jar dependency to 1.0.1

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80720194 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80718914 --- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/PacemakerClientPool.java --- @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80718833 --- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/PacemakerClientPool.java --- @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80718589 --- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/PacemakerClientHandler.java --- @@ -69,7 +69,7 @@ else if(evm instanceof HBMessage) {

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80719082 --- Diff: storm-core/src/jvm/org/apache/storm/pacemaker/PacemakerClientPool.java --- @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #1710: STORM-1546: Adding Read and Write Aggregations for...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1710#discussion_r80717469 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -926,10 +926,10 @@ public static final String UI_HTTPS_NEED_CLIENT_AUTH =

[GitHub] storm issue #1714: STORM-2125 Use Calcite's implementation of Rex Compiler

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1714 I'm trying to address our concern of nested map / array since Calcite community (Julian) says it sounds like a bug. Addressing it from CALCITE-1386 :

[GitHub] storm pull request #1713: Storm 2124 show requested cpu mem for each compone...

2016-09-27 Thread abellina
Github user abellina commented on a diff in the pull request: https://github.com/apache/storm/pull/1713#discussion_r80715643 --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj --- @@ -1016,6 +1019,10 @@ "name" (.get_topology_name comp-page-info)

[GitHub] storm pull request #1713: Storm 2124 show requested cpu mem for each compone...

2016-09-27 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1713#discussion_r80714135 --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj --- @@ -1016,6 +1019,10 @@ "name" (.get_topology_name comp-page-info)

[GitHub] storm issue #1715: Move new integration test to another build matrix

2016-09-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1715 @revans2 Since this is a small fix I didn't file it. Thanks for reviewing and merging! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] storm issue #1715: Move new integration test to another build matrix

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1715 @HeartSaVioR I merged this in, but didn't see a JIRA, if there is one please resolve it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] storm pull request #1715: Move new integration test to another build matrix

2016-09-27 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1715 --- 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

[GitHub] storm issue #1715: Move new integration test to another build matrix

2016-09-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1715 +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

[GitHub] storm issue #1470: STORM-1886 Extend KeyValueState iface with delete

2016-09-27 Thread kosii
Github user kosii commented on the issue: https://github.com/apache/storm/pull/1470 @HeartSaVioR I addressed most of the comments in my local repo, but not pushed yet. If I have some time this week (probably Friday) I'll clean it up, and commit. Sorry for the delay --- If your