Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r160513274
--- Diff: storm-core/src/clj/org/apache/storm/daemon/executor.clj ---
@@ -446,7 +451,7 @@
(.ack spout msg-id)
(task/apply-hooks (:user-context task-data) .spoutAck (SpoutAckInfo.
msg-id task-id time-delta))
(when time-delta
- (stats/spout-acked-tuple! (:stats executor-data) (:stream
tuple-info) time-delta))))
+ (stats/spout-acked-tuple! (:stats executor-data)
(StormMetricRegistry/counter "acked" (:worker-context executor-data)
(:component-id executor-data) (pr-str (:executor-id executor-data)) (:stream
tuple-info)) (:stream tuple-info) time-delta))))
--- End diff --
It's not ideal. Here's some background from earlier review:
@HeartSaVioR:
>> Looks like we compose metric name and lookup from REGISTRY every time,
even without executor ID and stream ID. I can see more calculation should be
done after addressing, but not sure how much it affects performance. If we
could also memorize metric name per combination of parameters it might help,
but I'm also not sure how much it will help.
@ptgoetz:
>Prior to adding stream ID, we could store the metric as a variable and
reuse it without having to do a lookup on each use. Adding stream ID required
(unless I'm missing something) doing the lookup on every use. There might be
additional optimizations, but because metrics names are composed of several
fields, some level of string concatenation is unavoidable. For example, we
could try to optimize lookups by caching metrics with metric name as the key,
but we would still have to do concatenation to create the lookup key.
>I suppose we could create a MetricName class to serve as the cache key,
but we'd be trading string concatenation for object creation (unless we name
MetricName mutable with setX methods).
---
