[STORM-3365] Configuration to disable Topology Lag Monitoring #2984
Conversation
| @@ -316,6 +316,12 @@ | |||
| @isBoolean | |||
| public static final String UI_DISABLE_HTTP_BINDING = "ui.disable.http.binding"; | |||
|
|
|||
| /** | |||
| * This controls wheather Storm UI would not monitor Spout lag. | |||
kishorvpatil
force-pushed
the
storm-3365
branch
from
e9809dc
to
1374166
Compare
| @@ -316,6 +316,12 @@ | |||
| @isBoolean | |||
| public static final String UI_DISABLE_HTTP_BINDING = "ui.disable.http.binding"; | |||
|
|
|||
| /** | |||
| * This controls whether Storm UI would not monitor Spout lag. | |||
There was a problem hiding this comment.
Nit: If I didn't already know about storm-kafka-monitor, this would be unclear to me. I would also be confused why this doesn't do anything for non-Kafka spouts. How about "This controls whether Storm UI displays spout lag for the Kafka spout"
| @@ -1618,7 +1619,8 @@ private static Double nullToZero(Double value) { | |||
| * @return getTopologyLag. | |||
| */ | |||
| public static Map<String, Map<String, Object>> getTopologyLag(StormTopology userTopology, Map<String,Object> config) { | |||
| return TopologySpoutLag.lag(userTopology, config); | |||
| Boolean disableLagMonitoring = ObjectReader.getBoolean(config.get(DaemonConfig.UI_DISABLE_SPOUT_LAG_MONITORING), false); | |||
There was a problem hiding this comment.
Nit: Consider adding the default to defaults.yaml instead of here. That way it's obvious to users what the default is.
There was a problem hiding this comment.
+1
Btw, my 2 cents, I feel the ideal direction is addressing STORM-3202. Kafka source metrics are already reporting these offsets to Nimbus, so once we integrate it to REST API we may get rid of topology lag monitor tool.
|
Agree with Jungtaek, getting rid of storm-kafka-monitor would be great. |
|
@HeartSaVioR I totally agree that we should use spout metrics published to nimbus and make it generalized UI display and REST API response. At that point we can get rid of |
| @@ -98,6 +98,7 @@ ui.header.buffer.bytes: 4096 | |||
| ui.http.creds.plugin: org.apache.storm.security.auth.DefaultHttpCredentialsPlugin | |||
| ui.pagination: 20 | |||
| ui.disable.http.binding: true | |||
| ui.disable.spout.lag.monitoring: true | |||
There was a problem hiding this comment.
@srdo I would rather turn off the feature by default. I can make it false if you think otherwise.
There was a problem hiding this comment.
No, it's fine by me. I just wanted to be sure it wasn't a mistake.
Sometimes Kafka broker is unreachable/security can cause issue..making UI hang for too long. So adding feature to allow disabling Topology Lag monitoring.