Skip to content

STORM-2452: Storm Metric classes are not thread safe#2041

Closed
knusbaum wants to merge 2 commits intoapache:masterfrom
knusbaum:STORM-2452
Closed

STORM-2452: Storm Metric classes are not thread safe#2041
knusbaum wants to merge 2 commits intoapache:masterfrom
knusbaum:STORM-2452

Conversation

@knusbaum
Copy link
Contributor

@knusbaum knusbaum commented Apr 3, 2017

Synchronizing metrics methods

@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented Apr 4, 2017

I think thread-unsafety of metrics is by design. They're guaranteed to be called only a thread at the same time, so unless we get or update the value on other than Spout or Bolt's event thread, they should be safe. Updating them is occurred from critical path, and metrics doesn't need to have synchronization overhead with current design.

Btw, thread-unsafety on metrics is why I gave up metrics aggregation for worker side. Instead of struggling about metrics more, I'm just waiting for Metrics V2.

@HeartSaVioR
Copy link
Contributor

Please do some benchmark and check performance regression, and please let me know if it doesn't hurt performance.

@knusbaum knusbaum closed this Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants