[
https://issues.apache.org/jira/browse/KAFKA-6819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16451257#comment-16451257
]
Guozhang Wang commented on KAFKA-6819:
--------------------------------------
Thanks for your feedbacks John.
For 2.a), the only reason I'm thinking of a static function is that in some
unit tests, the StreamThread's StreamThreadMetrics may not be initialized while
some other classes may want to access to its sensors. In this case we may
indeed want to lazily create the sensors. Your proposal differs with mine in
the sense that you're suggesting to bookkeep all the ever created sensors in
SMI, while I was proposing to bookkeep all the ever created sensors at each
layer in its corresponding XMetrics class. While reviewing your PR 4917 I think
your proposal maybe better, since then we only need users to access one class:
StreamsMetrics (and internally SMI) to cleanup any sensors ever created, either
during normal runs or during unit testing.
> Refactor build-in StreamsMetrics internal implementations
> ---------------------------------------------------------
>
> Key: KAFKA-6819
> URL: https://issues.apache.org/jira/browse/KAFKA-6819
> Project: Kafka
> Issue Type: Improvement
> Components: streams
> Reporter: Guozhang Wang
> Priority: Major
>
> Our current internal implementations of StreamsMetrics and different layered
> metrics like StreamMetricsThreadImpl, TaskMetrics, NodeMetrics etc are a bit
> messy nowadays. We could improve on the current situation by doing the
> following:
> 0. For thread-level metrics, refactor the {{StreamsMetricsThreadImpl}} class
> to {{ThreadMetrics}} such that a) it does not extend from
> {{StreamsMetricsImpl}} but just include the {{StreamsMetricsThreadImpl}} as
> its constructor parameters. And make its constructor, replacing with a static
> {{addAllSensors(threadName)}} that tries to register all the thread-level
> sensors for the given thread name.
> 1. Add a static function for each of the built-in sensors of the thread-level
> metrics in {{ThreadMetrics}} that relies on the internal
> {{StreamsMetricsConventions}} to get thread level sensor names. If the sensor
> cannot be found from the internal {{Metrics}} registry, create the sensor
> on-the-fly.
> 2.a Add a static {{removeAllSensors(threadName)}} function in
> {{ThreadMetrics}} that tries to de-register all the thread-level metrics for
> this thread, if there is no sensors then it will be a no-op. In
> {{StreamThread#close()}} we will trigger this function; and similarly in
> `TopologyTestDriver` when we close the driver we will also call this function
> as well. As a result, the {{ThreadMetrics}} class itself would only contain
> static functions with no member fields at all.
> 2.b We can consider doing the same for {{TaskMetrics}}, {{NodeMetrics}} and
> {{NamedCacheMetrics}} as well, and add a {{StoreMetrics}} following the
> similar pattern: although these metrics are not accessed externally to their
> enclosing class in the future this may be changed as well.
> 3. Then, we only pass {{StreamsMetricsImpl}} around between the internal
> classes, to access the specific sensor whenever trying to record it.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
