Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2794
Okay, just wanted to be sure it wasn't an oversight. +1.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2794
@srdo No, unfortunately we don't apply consistent naming so
`totalExecutors` is right for others. Renaming them would break clients which
rely on the REST API, so I'm not sure we would be better
Github user govind-menon commented on the issue:
https://github.com/apache/storm/pull/2795
@HeartSaVioR Fix for 3182. Working on the others.
---
GitHub user govind-menon opened a pull request:
https://github.com/apache/storm/pull/2795
STORM-3182: Removing superfluous topo id check from owner resource APâ¦
â¦I request
You can merge this pull request into a Git repository by running:
$ git pull
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208012336
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -53,12 +55,14 @@ public static Meter registerMeter(String
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208010887
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -53,12 +55,14 @@ public static Meter registerMeter(String name) {
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208010012
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -53,12 +55,14 @@ public static Meter registerMeter(String
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208008824
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/metrics/reporters/PreparableReporter.java
---
@@ -13,16 +13,35 @@
package
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2789
The exit code is 20. Try looking for uses of `Utils.exitProcess` with `20`
as the argument. If you change the values to be unique and rerun the tests, you
should be able to nail down where the exit is
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208007711
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -53,12 +55,14 @@ public static Meter registerMeter(String name) {
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208005815
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
---
@@ -446,6 +447,7 @@ public void
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2789#discussion_r208005061
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/metrics/reporters/PreparableReporter.java
---
@@ -13,16 +13,35 @@
package
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2789
Build logs report test crash [INFO] Running
org.apache.storm.scheduler.resource.TestResourceAwareScheduler, but it's
passing on my local machine. Don't know what's happening. It'll be great if
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2789
https://issues.apache.org/jira/browse/STORM-3128 This bug in test appeared
again and broke the test in this PR but I don't actually know the cause of it.
---
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2764
This can be merged after #2789
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2794
Grepping for "totalExecutors" also pops up a couple of other locations in
storm-webapp.
```
./storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java:536:
int
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2783
Thanks
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2771
+1, though maybe some of this can be made unnecessary in the future if we
decide to make StormMetricsRegistry non-static.
Thanks for your patience @zd-project, merged to master.
---
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2783
I'm really sorry about the timing. I'll try to take a look at it once the
merge is done.
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2771
---
Github user srdo closed the pull request at:
https://github.com/apache/storm/pull/2783
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2783
I'll close this in the meantime.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2783
@zd-project Alright, maybe the best option is that we postpone looking at
this until the added metrics have been merged.
---
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2771
Squashed.
---
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2783
I can port changes on top of this and open another PR, but I hope existing
PRs can be merged in first, as they provide actual metrics that are straight
useful to system administrators and
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2790
---
GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/storm/pull/2794
STORM-3180 Total executors in Cluster Summary in main UI page is not â¦
â¦exposed even a topology is running
* rename the field of /cluster/summary output: `totalExecutors` to
Github user dfdemar commented on the issue:
https://github.com/apache/storm/pull/2790
@srdo Ready to merge!
---
28 matches
Mail list logo
