Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4840#discussion_r146582951
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/legacy/metrics/MetricStore.java
---
@@ -260,50 +293,66 @@ public String getMetric(String name, String
defaultValue) {
? value
: defaultValue;
}
- }
- /**
- * Sub-structure containing metrics of the JobManager.
- */
- public static class JobManagerMetricStore extends ComponentMetricStore {
+ public static ComponentMetricStore
unmodifiable(ComponentMetricStore source) {
+ if (source == null) {
+ return null;
+ }
+ return new
ComponentMetricStore(unmodifiableMap(source.metrics));
+ }
}
/**
* Sub-structure containing metrics of a single TaskManager.
*/
+ @ThreadSafe
public static class TaskManagerMetricStore extends ComponentMetricStore
{
- public final Set<String> garbageCollectorNames = new
HashSet<>();
+ public final Set<String> garbageCollectorNames;
+
+ public TaskManagerMetricStore() {
+ this(new ConcurrentHashMap<>(),
ConcurrentHashMap.newKeySet());
+ }
+
+ public TaskManagerMetricStore(Map<String, String> metrics,
Set<String> garbageCollectorNames) {
+ super(metrics);
+ this.garbageCollectorNames =
checkNotNull(garbageCollectorNames);
+ }
public void addGarbageCollectorName(String name) {
garbageCollectorNames.add(name);
}
+
+ public static TaskManagerMetricStore
unmodifiable(TaskManagerMetricStore source) {
--- End diff --
In which case we should rather modify the store to not allow writes in the
first place, instead of opting for unmodifiable collections that are pretty
much a hack. "here, have an object that fails for 50% of the defined methods";
that's hardly good design is it.
Till suggested dedicated read methods that return metrics, something like
`List<String> getMetrics(List<String> metricNames)` instead of exposing the
metric maps.
This would make the interface cleaner and would allow us to simplify the
synchronization.
---
