[jira] [Comment Edited] (STORM-2600) Improve or replace storm-kafka-monitor

["Stig Rohde Døssing (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/STORM-2600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16062565#comment-16062565 ] 

Stig Rohde Døssing edited comment on STORM-2600 at 6/26/17 6:14 AM:
--------------------------------------------------------------------

[~pshah] That doesn't seem to match the behavior I'm seeing though. It looks to me like the getTopicsString value is put into the component configuration once, and then never updated, which is why the lag endpoint requests offsets for an empty topic list in the test I mentioned in this comment https://github.com/apache/storm/pull/2150#discussion_r123901054.

The lag endpoint uses the StormTopology ComponentCommon to read the getTopicsString value https://github.com/apache/storm/blob/64e29f365c9b5d3e15b33f33ab64e200345333e4/storm-core/src/jvm/org/apache/storm/utils/TopologySpoutLag.java#L143. As far as I can tell, that value is set here https://github.com/apache/storm/blob/9e31509d47c4e91c1009f55c7ccf321d7d7e63aa/storm-client/src/jvm/org/apache/storm/topology/TopologyBuilder.java#L542, getting fetched through getComponentConfiguration where the spout sets the topic list https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java#L525.  

The problem is that the StormTopology ComponentCommon value doesn't seem to be updated after the topology is built, so it doesn't matter what getTopicsString returns once the spout is up and running, because it'll never be propagated to the lag endpoint.

About point 2, the worry is a minor one, but my concern isn't that the Kafka API breaks, since we use such a small part of it for storm-kafka-monitor. My concern is that the KafkaConsumer is updated to use new request versions (e.g. FetchRequest) toward Kafka. Generally older request versions are only supported for one release. In this case you wouldn't necessarily have to make any changes to your topology jar beyond replacing the client library, but you would have to rebuild storm-kafka-monitor since the jar is shaded in. Like I said, it's a fairly minor thing.

About 3, I agree that it is possible to work around, and that there probably aren't that many Windows users. I just thought it was worth mentioning, since this is a case where there is no intrinsic reason we can't support Windows (unlike for example cgroups support), so I see it as a minor disadvantage to doing offset lag tracking this way.

I agree that it would be good to have some opinions on whether a metrics-based solution would be desirable. My main concern about the current implementation is that I don't think point 1 works. The other two are just nitpicking to some extent.


was (Author: srdo):
@priyank5485 That doesn't seem to match the behavior I'm seeing though. It looks to me like the getTopicsString value is put into the component configuration once, and then never updated, which is why the lag endpoint requests offsets for an empty topic list in the test I mentioned in this comment https://github.com/apache/storm/pull/2150#discussion_r123901054.

The lag endpoint uses the StormTopology ComponentCommon to read the getTopicsString value https://github.com/apache/storm/blob/64e29f365c9b5d3e15b33f33ab64e200345333e4/storm-core/src/jvm/org/apache/storm/utils/TopologySpoutLag.java#L143. As far as I can tell, that value is set here https://github.com/apache/storm/blob/9e31509d47c4e91c1009f55c7ccf321d7d7e63aa/storm-client/src/jvm/org/apache/storm/topology/TopologyBuilder.java#L542, getting fetched through getComponentConfiguration where the spout sets the topic list https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java#L525.  

The problem is that the StormTopology ComponentCommon value doesn't seem to be updated after the topology is built, so it doesn't matter what getTopicsString returns once the spout is up and running, because it'll never be propagated to the lag endpoint.

About point 2, the worry is a minor one, but my concern isn't that the Kafka API breaks, since we use such a small part of it for storm-kafka-monitor. My concern is that the KafkaConsumer is updated to use new request versions (e.g. FetchRequest) toward Kafka. Generally older request versions are only supported for one release. In this case you wouldn't necessarily have to make any changes to your topology jar beyond replacing the client library, but you would have to rebuild storm-kafka-monitor since the jar is shaded in. Like I said, it's a fairly minor thing.

About 3, I agree that it is possible to work around, and that there probably aren't that many Windows users. I just thought it was worth mentioning, since this is a case where there is no intrinsic reason we can't support Windows (unlike for example cgroups support), so I see it as a minor disadvantage to doing offset lag tracking this way.

I agree that it would be good to have some opinions on whether a metrics-based solution would be desirable. My main concern about the current implementation is that I don't think point 1 works. The other two are just nitpicking to some extent.

> Improve or replace storm-kafka-monitor
> --------------------------------------
>
>                 Key: STORM-2600
>                 URL: https://issues.apache.org/jira/browse/STORM-2600
>             Project: Apache Storm
>          Issue Type: Improvement
>          Components: storm-kafka-monitor
>    Affects Versions: 2.0.0
>            Reporter: Stig Rohde Døssing
>            Priority: Minor
>
> The storm-kafka-monitor module, which is used by Storm UI to show offset lag for topologies with Kafka spouts, has some shortcomings:
> * The Storm UI integration code doesn't seem to be able to support topic subscriptions that change after topology submission. The UI code (https://github.com/apache/storm/blob/64e29f365c9b5d3e15b33f33ab64e200345333e4/storm-core/src/jvm/org/apache/storm/utils/TopologySpoutLag.java#L91) gets the topic list it should request offset lag for via the spout's getComponentConfiguration method, as far as I can tell through this call https://github.com/apache/storm/blob/9e31509d47c4e91c1009f55c7ccf321d7d7e63aa/storm-client/src/jvm/org/apache/storm/topology/TopologyBuilder.java#L541. It seems like the component configuration is intended to be static once the topology has started running. This prevents us from showing the right topic list for subscriptions that are not known at submission time, which is currently the case for Pattern subscriptions. The topic list for that type of subscription isn't known until the spout has started the KafkaConsumer in {{ISpout.open()}}. I don't see a way to fix this, unless there is some way to update the component configuration when the subscription changes.
> * The jar is installed along with the cluster, and depends on the Kafka version specified in Storm's root POM. Kafka guarantees backwards compatible client-server communication for one release only, so there's a potential coupling between Storm cluster version and Kafka version. If users want to update the Kafka version in storm-kafka-monitor, they have to rebuild that module and replace the jar in their Storm install.
> * The UI integration uses the storm-kafka-monitor Bash script to start the monitoring code, in order to avoid a dependency between storm-core and storm-kafka-monitor. This prevents the UI integration from working on Windows. We could supply a Windows script as well, but then we'd need to keep the two in sync.
> I am wondering if these problems could be solved by implementing offset lag monitoring via the metrics system instead. The spout could periodically seek to the log end offset and submit a metric for how far behind the committed offset is, then seek back to where it left off.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)