Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2300#discussion_r75152566
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java
---
@@ -83,9 +88,25 @@
//
------------------------------------------------------------------------
- public AbstractMetricGroup(MetricRegistry registry, String[] scope) {
+ public AbstractMetricGroup(MetricRegistry registry, String[] scope, A
parent) {
this.registry = checkNotNull(registry);
this.scopeComponents = checkNotNull(scope);
+ this.parent = parent;
+ }
+
+ public Map<String, String> getAllVariables() {
+ if (variables == null) { // avoid synchronization for common
case
--- End diff --
I think this goes into a tricky realm. Judging the consequences of these
loose concurrency tricks is quite hard.
In your suggestion, I think that if isEmpty() is false, it may still be
being filled, and then other accessors get concurrent modification exceptions.
Who accesses the `volatile` variable? The reporter threads? In that case,
the minimal performance hit is not too bad (it would not be in the data
processing threads). If the `getAllVariables()` function is actually called on
every group anyways for the most common reporter (JMX), then we might just as
well build the map eagerly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
