Improvements to FateMetrics

apache / accumulo

Apache Accumulo

https://accumulo.apache.org

Apache License 2.0

1.07k stars 445 forks source link

Improvements to FateMetrics #4924

Closed DomGarguilo closed 1 month ago

DomGarguilo commented 1 month ago

In this PR

refactored FateMetrics to use an EnumMap for TStatus metric counters
- this allows us to avoid coverting to/from strings for the TStatuses
- we can simplify the code to loop through this map when updating or constructing these metrics
refactored the map of txStateCounters in FateMetricValues to use an EnumMap as well. FateMetrics is the only spot where this map is retrieved so we simplify converting to/from Strings there too.

DomGarguilo commented 1 month ago

Something that I noticed while working on these changes was the following: https://github.com/apache/accumulo/blob/88bd23ea5afd424242e95ff3963f0d898368bd5e/server/manager/src/main/java/org/apache/accumulo/manager/metrics/fate/FateMetrics.java#L114-L115 I haven't looked into things too much but it seems like this code would create a new micrometer Gauge each time it is called (which ends up being once every second).

DomGarguilo commented 1 month ago

Something that I noticed while working on these changes was the following:

https://github.com/apache/accumulo/blob/88bd23ea5afd424242e95ff3963f0d898368bd5e/server/manager/src/main/java/org/apache/accumulo/manager/metrics/fate/FateMetrics.java#L114-L115

I haven't looked into things too much but it seems like this code would create a new micrometer Gauge each time it is called (which ends up being once every second).

Made a ticket to track this potential issue.