You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2021/05/07 22:13:05 UTC

[brooklyn-server] branch master updated: preserve order of config keys on the entity type

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git


The following commit(s) were added to refs/heads/master by this push:
     new e99ea54  preserve order of config keys on the entity type
e99ea54 is described below

commit e99ea548045768cd325a156f0ae47b42331cc636
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Fri May 7 22:34:40 2021 +0100

    preserve order of config keys on the entity type
    
    replace ConcurrentHashMap with LinkedHashMap and use lock to guard access
---
 .../core/enricher/EnricherDynamicType.java         |  2 +-
 .../brooklyn/core/entity/EntityDynamicType.java    | 14 +++++---
 .../location/internal/LocationDynamicType.java     |  2 +-
 .../brooklyn/core/objs/BrooklynDynamicType.java    | 37 ++++++++++++++++------
 .../brooklyn/core/policy/PolicyDynamicType.java    |  2 +-
 5 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/enricher/EnricherDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/enricher/EnricherDynamicType.java
index 4331ee0..52aa8bb 100644
--- a/core/src/main/java/org/apache/brooklyn/core/enricher/EnricherDynamicType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/enricher/EnricherDynamicType.java
@@ -39,6 +39,6 @@ public class EnricherDynamicType extends BrooklynDynamicType<Enricher, AbstractE
 
     @Override
     protected EnricherTypeSnapshot newSnapshot() {
-        return new EnricherTypeSnapshot(name, value(configKeys));
+        return new EnricherTypeSnapshot(name, getConfigKeysModifiableCopy());
     }
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java
index a0dfa5f..94f4aec 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java
@@ -232,7 +232,7 @@ public class EntityDynamicType extends BrooklynDynamicType<Entity, AbstractEntit
      * Adds the given {@link ConfigKey} to this entity.
      */
     public void addConfigKey(ConfigKey<?> newKey) {
-        configKeys.put(newKey.getName(), new FieldAndValue<ConfigKey<?>>(null, newKey));
+        withLock(configKeysRW.writeLock(), () -> configKeysNCC.put(newKey.getName(), new FieldAndValue<ConfigKey<?>>(null, newKey)));
         invalidateSnapshot();
         instance.sensors().emit(AbstractEntity.CONFIG_KEY_ADDED, newKey);
     }
@@ -250,7 +250,7 @@ public class EntityDynamicType extends BrooklynDynamicType<Entity, AbstractEntit
      * Removes the named {@link ConfigKey} from this entity.
      */
     public boolean removeConfigKey(ConfigKey<?> key) {
-        FieldAndValue<ConfigKey<?>> result = configKeys.remove(key.getName());
+        FieldAndValue<ConfigKey<?>> result = withLock(configKeysRW.writeLock(), () -> configKeysNCC.remove(key.getName()));
         if (result != null) {
             invalidateSnapshot();
             ConfigKey<?> removedKey = result.value;
@@ -262,8 +262,12 @@ public class EntityDynamicType extends BrooklynDynamicType<Entity, AbstractEntit
     }
     
     public void clearConfigKeys() {
-        Map<String, FieldAndValue<ConfigKey<?>>> oldKeys = MutableMap.copyOf(configKeys);
-        configKeys.clear();
+        Map<String, FieldAndValue<ConfigKey<?>>> oldKeys = MutableMap.of();
+        withLock(configKeysRW.writeLock(), () -> {
+            oldKeys.putAll(configKeysNCC);
+            configKeysNCC.clear();
+            return null;
+        });
         invalidateSnapshot();
         for (FieldAndValue<ConfigKey<?>> k: oldKeys.values()) {
             instance.sensors().emit(AbstractEntity.CONFIG_KEY_REMOVED, k.value);
@@ -274,7 +278,7 @@ public class EntityDynamicType extends BrooklynDynamicType<Entity, AbstractEntit
     
     @Override
     protected EntityTypeSnapshot newSnapshot() {
-        return new EntityTypeSnapshot(name, value(configKeys), sensors, effectors.values());
+        return new EntityTypeSnapshot(name, getConfigKeysModifiableCopy(), sensors, effectors.values());
     }
     
     /**
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationDynamicType.java
index 43c126c..b17c347 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationDynamicType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationDynamicType.java
@@ -36,6 +36,6 @@ public class LocationDynamicType extends BrooklynDynamicType<Location, AbstractL
 
     @Override
     protected LocationTypeSnapshot newSnapshot() {
-        return new LocationTypeSnapshot(name, value(configKeys));
+        return new LocationTypeSnapshot(name, getConfigKeysModifiableCopy());
     }
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynDynamicType.java
index d183c2c..69f8ec2 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynDynamicType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynDynamicType.java
@@ -30,9 +30,14 @@ import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
 import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.objs.BrooklynType;
 import org.apache.brooklyn.config.ConfigInheritances;
@@ -70,7 +75,8 @@ public abstract class BrooklynDynamicType<T extends BrooklynObject, AbstractT ex
     /** 
      * Map of config keys (and their fields) on this instance, by name.
      */
-    protected final Map<String,FieldAndValue<ConfigKey<?>>> configKeys = new ConcurrentHashMap<String, FieldAndValue<ConfigKey<?>>>();
+    protected final Map<String,FieldAndValue<ConfigKey<?>>> configKeysNCC = new LinkedHashMap<String, FieldAndValue<ConfigKey<?>>>();
+    protected final transient ReadWriteLock configKeysRW = new ReentrantReadWriteLock();
 
     private volatile BrooklynTypeSnapshot snapshot;
     private final AtomicBoolean snapshotValid = new AtomicBoolean(false);
@@ -88,9 +94,9 @@ public abstract class BrooklynDynamicType<T extends BrooklynObject, AbstractT ex
         // NB: official name is usually injected later, e.g. from AbstractEntity.setManagementContext
         this.name = (clazz.getCanonicalName() == null) ? clazz.getName() : clazz.getCanonicalName();
         
-        buildConfigKeys(clazz, null, configKeys);
+        buildConfigKeys(clazz, null, configKeysNCC);
         if (LOG.isTraceEnabled())
-            LOG.trace("Entity {} config keys: {}", (instance==null ? clazz.getName() : instance.getId()), Joiner.on(", ").join(configKeys.keySet()));
+            LOG.trace("Entity {} config keys: {}", (instance==null ? clazz.getName() : instance.getId()), Joiner.on(", ").join(configKeysNCC.keySet()));
     }
     
     protected abstract BrooklynTypeSnapshot newSnapshot();
@@ -116,24 +122,28 @@ public abstract class BrooklynDynamicType<T extends BrooklynObject, AbstractT ex
     }
     
     // --------------------------------------------------
-    
+
+    protected Map<String,ConfigKey<?>> getConfigKeysModifiableCopy() {
+        return withLock(configKeysRW.readLock(), () -> value(configKeysNCC));
+    }
+
     /**
      * ConfigKeys available on this entity.
      */
     public Map<String,ConfigKey<?>> getConfigKeys() {
-        return Collections.unmodifiableMap(value(configKeys));
+        return Collections.unmodifiableMap(getConfigKeysModifiableCopy());
     }
 
     /**
      * ConfigKeys available on this entity.
      */
-    public ConfigKey<?> getConfigKey(String keyName) { 
-        return value(configKeys.get(keyName)); 
+    public ConfigKey<?> getConfigKey(String keyName) {
+        return value(withLock(configKeysRW.readLock(), () -> configKeysNCC.get(keyName)));
     }
 
     /** field where a config key is defined, for use getting annotations. note annotations are not inherited. */
-    public Field getConfigKeyField(String keyName) { 
-        return field(configKeys.get(keyName)); 
+    public Field getConfigKeyField(String keyName) {
+        return field(withLock(configKeysRW.readLock(), () -> configKeysNCC.get(keyName)));
     }
 
     protected BrooklynTypeSnapshot refreshSnapshot() {
@@ -292,4 +302,13 @@ public abstract class BrooklynDynamicType<T extends BrooklynObject, AbstractT ex
         return result;
     }
 
+    protected static <T> T withLock(Lock lock, Supplier<T> call) {
+        try {
+            lock.lock();
+            return call.get();
+        } finally {
+            lock.unlock();
+        }
+    }
+
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/policy/PolicyDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/policy/PolicyDynamicType.java
index d4a9a68..22eb243 100644
--- a/core/src/main/java/org/apache/brooklyn/core/policy/PolicyDynamicType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/policy/PolicyDynamicType.java
@@ -39,6 +39,6 @@ public class PolicyDynamicType extends BrooklynDynamicType<Policy, AbstractPolic
 
     @Override
     protected PolicyTypeSnapshot newSnapshot() {
-        return new PolicyTypeSnapshot(name, value(configKeys));
+        return new PolicyTypeSnapshot(name, getConfigKeysModifiableCopy());
     }
 }