flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zentol <...@git.apache.org>
Subject [GitHub] flink pull request #4840: [FLINK-7368][metrics] Make MetricStore ThreadSafe ...
Date Tue, 24 Oct 2017 13:10:52 GMT
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4840#discussion_r146553570
  
    --- 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 --
    
    I'm wondering whether we really need this unmodifiable business. Yes, it's technically
a good idea, but the access to the MetricStore is limited and fully under our control; so
we _know_ that we never try to modify the map.


---

Mime
View raw message