Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2783
I think it depends a lot on what kind of static method we're talking about.
If the method is internal we can just change it and inject the metric, or even
better make the method not static and inject
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2783
Additionally, it still seems to be not ideal that if we want to measure
things inside a static method, we'd have to pass the metric in as a parameter.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2783
@zd-project You are right that if someone wants to add metrics to a
component, they will have to figure out how to get the registry injected. I
can't say up front how hard that will be, except to note
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207672905
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -12,28 +12,30 @@
package org.apache.storm.metric;
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207671937
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -62,6 +62,7 @@
private OutputCollector
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2754
The metrics of IO exceptions on logviewer is kind of fuzzy as I go through
the call stack manually and eyeballed all thrown exceptions. I'm not sure if
it's complete or if it will over-count.
Github user Ethanlm commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207665264
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -50,19 +61,19 @@ public static void startMetricsReporters(Map
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207663344
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -50,19 +61,19 @@ public static void startMetricsReporters(Map
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2752
---
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2752
Please squash the commits.
---
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207649108
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -62,6 +62,7 @@
private OutputCollector
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2710
Should I squash to one per daemon?
---
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2710#discussion_r207645153
--- Diff:
storm-client/src/jvm/org/apache/storm/daemon/supervisor/ClientSupervisorUtils.java
---
@@ -28,11 +29,14 @@
import
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2783
Extensibility and Centralized management would be my concerns. I think this
PR has improved on the centralized management of metrics. For example I think
the slotMetrics class is a great example
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207640687
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -12,28 +12,30 @@
package
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207638261
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -12,28 +12,30 @@
package
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207636496
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -50,19 +61,19 @@ public static void
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207601720
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -12,28 +12,30 @@
package org.apache.storm.metric;
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207599113
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -12,28 +12,30 @@
package
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2788
---
Github user Ethanlm commented on a diff in the pull request:
https://github.com/apache/storm/pull/2771#discussion_r207594931
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/StormMetricsRegistry.java ---
@@ -50,19 +61,19 @@ public static void startMetricsReporters(Map
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2788
Still +1
---
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2788#discussion_r207587787
--- Diff:
storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/testsupport/MockRemovableFileBuilder.java
---
@@ -0,0 +1,29 @@
+/**
+ *
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2788#discussion_r207587402
--- Diff:
storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/testsupport/MockRemovableFileBuilder.java
---
@@ -0,0 +1,29 @@
+/**
+ *
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2787
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2785
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2784
---
Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2788#discussion_r207586437
--- Diff:
storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/testsupport/MockRemovableFileBuilder.java
---
@@ -0,0 +1,29 @@
+/**
+ *
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2787
Squashed.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2788
+1, thanks for fixing the issue with nondeleteable files. The solution
looks good. Please squash and we can merge.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2787
+1, thanks for the quick fix. Please squash and we can merge.
---
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2786
---
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2710
Also could we squash the commits some?
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2710#discussion_r207578433
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java ---
@@ -1147,8 +1162,19 @@ public DynamicState
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2710#discussion_r207576736
--- Diff:
storm-client/src/jvm/org/apache/storm/daemon/supervisor/ClientSupervisorUtils.java
---
@@ -28,11 +29,14 @@
import
Github user zd-project commented on the issue:
https://github.com/apache/storm/pull/2771
`name()` has been removed from this PR.
---
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207560866
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -129,6 +140,27 @@ public void prepare(Map topoConf,
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2752#discussion_r207554122
--- Diff:
storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java ---
@@ -0,0 +1,1941 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2752#discussion_r207551893
--- Diff:
storm-webapp/src/main/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilter.java
---
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2752
I may have found out more. In the logs I see
```
Caused by: java.io.IOException: Unable to fetch topo conf for topo id
wc-1-1533264135
at
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2752
I am seeing these when I run DRPC and the logviewer. They appear to still
work so this is minor, but it would be nice to fix them in a follow on JIRA if
you cannot do it quickly now.
```
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2752#discussion_r207542398
--- Diff:
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java
---
@@ -137,7 +137,7 @@ public Response
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2788#discussion_r207539609
--- Diff:
storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/testsupport/MockRemovableFileBuilder.java
---
@@ -0,0 +1,29 @@
+/**
+ *
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207537361
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -62,6 +62,7 @@
private OutputCollector
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207534991
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -129,6 +140,27 @@ public void prepare(Map topoConf,
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207526335
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -37,38 +39,48 @@
import
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207524658
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -37,38 +39,48 @@
import
Github user dfdemar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207524275
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -37,38 +39,48 @@
import
Github user wangzzu commented on the issue:
https://github.com/apache/storm/pull/2791
@srdo I found that we are using the deprecated Subscription subtypes
actually, thx.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2791
@wangzzu Yes, but it should also have been fixed in 1.1.2. Please make sure
you're not using one of the deprecated Subscription subtypes, e.g.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207511238
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -62,6 +62,7 @@
private OutputCollector
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207515186
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -37,38 +39,48 @@
import
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207512766
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -62,6 +62,7 @@
private OutputCollector
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207514281
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java
---
@@ -37,38 +39,48 @@
import
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2790#discussion_r207511912
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java
---
@@ -129,6 +140,27 @@ public void prepare(Map topoConf,
Github user wangzzu closed the pull request at:
https://github.com/apache/storm/pull/2791
---
Github user wangzzu commented on the issue:
https://github.com/apache/storm/pull/2791
@srdo sorry, the version I used is 1.1.2. This problem has already fixed in
1.2.0.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2791
@wangzzu Hi, I'm not sure this should be happening. We're not using the
`KafkaConsumer.subscribe` API anymore, so Kafka shouldn't be managing the
partition assignment.
Can you confirm that
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2784
Yes, exactly.
---
GitHub user wangzzu opened a pull request:
https://github.com/apache/storm/pull/2791
STORM-3176: KafkaSpout commit offset occurs CommitFailedException which
leads to worker dead
KafkaSpout use the commitAsync api of Consumer, if the interval time
between call consumer.poll() more
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2783
I love all the approaches to switch static to non-static if it doesn't
require any hack or long parameters to inject, and this looks great. +1 to move
on.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2773
@jiangzhileaf Any updates?
---
62 matches
Mail list logo
