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

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 I also want to make sure that we take the performance impact of this patch into consideration. With the old metrics system we are sub-sampling the metrics for performance reasons. With this patch

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

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 @ptgoetz I think we can fix most of the issues simply by restricting what name a metric can have (i.e. no `:` allowed and then we use `:` as a deliminator). That will give us a guarantee on being

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

2017-09-29 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/2203 @revans2 Added rudimentary sanity check validation for metrics reporters configs. Because reporter implementations may want to have their own custom config keys, we can't really cover everything. I

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

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 So to parse these pieces of information back out I would need to do something like remove "storm.topology." from the beginning. Then everything up to the next "." would be the topology-ID

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

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 So then perhaps I can summarize what we have right now for the naming. ```"storm.topology.%s.%s.%s-%s", getStormId(), getThisComponentId(), getThisWorkerPort(), name``` Where

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

2017-09-29 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/2203 @revans2 I agree with you regarding getting the metrics naming correct. In fact I think it's one of the most important aspects. I'd like to get as much input/collaboration from others as possible to

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

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

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

2017-09-29 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r141933736 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java --- @@ -0,0 +1,137 @@ +/** + * Licensed to the Apache Software

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

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 @danny0405 @abellina actually has a patch to do what you were asking about. To push metrics to nimbus by way of the supervisor. It is obviously based off of this patch, so a lot of this is waiting

[GitHub] storm pull request #2347: STORM-2760: Add Blobstore Migration Scripts

2017-09-29 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2347 ---

[GitHub] storm issue #2347: STORM-2760: Add Blobstore Migration Scripts

2017-09-29 Thread knusbaum
Github user knusbaum commented on the issue: https://github.com/apache/storm/pull/2347 integration test failure looks unrelated. ---

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

2017-09-29 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/2203 @revans2, @HeartSaVioR, @arunmahadevan Addressed review comments. Also note that this is a feature branch, so it's open to pull requests from anyone. If there's anything you'd like to see

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

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

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

2017-09-29 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r141922507 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java --- @@ -0,0 +1,135 @@ +/** + * Licensed to the Apache Software

[GitHub] storm issue #2345: STORM-2438: added in rebalance changes to support RAS

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2345 For anyone interested I just rebased as #2339 was merged into master. ---

[GitHub] storm issue #2350: STORM-2764: Only login the user if not already done

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2350 @HeartSaVioR I put it up as #2352 ---

[GitHub] storm issue #2352: STORM-2764: Only login the user if not already done (1.x)

2017-09-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2352 This is the 1.x version of #2350 ---

[GitHub] storm pull request #2352: STORM-2764: Only login the user if not already don...

2017-09-29 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2352 STORM-2764: Only login the user if not already done You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2764-1.x

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

2017-09-29 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1674 ---

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

2017-09-29 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2343 ---