From 2fb2e0555abdfee70d1171f0375ef93334b00eca Mon Sep 17 00:00:00 2001 From: zentol Date: Mon, 7 May 2018 10:04:26 +0200 Subject: [PATCH 1/2] [FLINK-9258][metrics] Thread-safe initialization of variables map --- .../flink/runtime/metrics/groups/ComponentMetricGroup.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java index 0cd9942e37a02..87c6a9a5185d2 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java @@ -57,11 +57,12 @@ public Map getAllVariables() { if (variables == null) { // avoid synchronization for common case synchronized (this) { if (variables == null) { - variables = new HashMap<>(); - putVariables(variables); + Map tmpVariables = new HashMap<>(); + putVariables(tmpVariables); if (parent != null) { // not true for Job-/TaskManagerMetricGroup - variables.putAll(parent.getAllVariables()); + tmpVariables.putAll(parent.getAllVariables()); } + variables = tmpVariables; } } } From 19d275459cb1cb1dbdea7ff2ca318177495c8c09 Mon Sep 17 00:00:00 2001 From: zentol Date: Wed, 23 May 2018 11:12:30 +0200 Subject: [PATCH 2/2] deduplicate methods; move fix to AbstractMetricGroup --- .../metrics/groups/AbstractMetricGroup.java | 9 ++++--- .../metrics/groups/ComponentMetricGroup.java | 27 ------------------- 2 files changed, 5 insertions(+), 31 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java index 6d9c7d95a3476..909915f216e99 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java @@ -113,11 +113,12 @@ public Map getAllVariables() { if (variables == null) { // avoid synchronization for common case synchronized (this) { if (variables == null) { - variables = new HashMap<>(); - putVariables(variables); - if (parent != null) { // not true for Job-/TaskManagerMetricGroup and mocks - variables.putAll(parent.getAllVariables()); + Map tmpVariables = new HashMap<>(); + putVariables(tmpVariables); + if (parent != null) { // not true for Job-/TaskManagerMetricGroup + tmpVariables.putAll(parent.getAllVariables()); } + variables = tmpVariables; } } } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java index 87c6a9a5185d2..924adcbf200ff 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java @@ -21,9 +21,6 @@ import org.apache.flink.annotation.Internal; import org.apache.flink.runtime.metrics.MetricRegistry; -import java.util.HashMap; -import java.util.Map; - /** * Abstract {@link org.apache.flink.metrics.MetricGroup} for system components (e.g., * TaskManager, Job, Task, Operator). @@ -52,30 +49,6 @@ public ComponentMetricGroup(MetricRegistry registry, String[] scope, P parent) { super(registry, scope, parent); } - @Override - public Map getAllVariables() { - if (variables == null) { // avoid synchronization for common case - synchronized (this) { - if (variables == null) { - Map tmpVariables = new HashMap<>(); - putVariables(tmpVariables); - if (parent != null) { // not true for Job-/TaskManagerMetricGroup - tmpVariables.putAll(parent.getAllVariables()); - } - variables = tmpVariables; - } - } - } - return variables; - } - - /** - * Enters all variables specific to this ComponentMetricGroup and their associated values into the map. - * - * @param variables map to enter variables and their values into - */ - protected abstract void putVariables(Map variables); - /** * Closes the component group by removing and closing all metrics and subgroups * (inherited from {@link AbstractMetricGroup}), plus closing and removing all dedicated