framework/config,server: configkey caching (#9628)

Added caching for ConfigKey value retrievals based on the Caffeine in-memory caching library. https://github.com/ben-manes/caffeine Currently, expire time for a cache is 30s and each update of the config key invalidates the cache. On any update or reset of the configuration, cache automatically invalidates for it. Signed-off-by: Abhishek Kumar <[email protected]>
apache · Sep 5, 2024 · 31b0ed0 · 31b0ed0
1 parent 787acfd
commit 31b0ed0
Show file tree

Hide file tree

Showing 19 changed files with 255 additions and 125 deletions.
diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
@@ -43,7 +43,6 @@
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
 import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.ScopedConfigStorage;
 import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -1256,12 +1255,10 @@ private void overrideVmMetadataConfigValue(final String manufacturer, final Stri
         ConfigKey configKey = VirtualMachineManager.VmMetadataManufacturer;
         this.configDepotImpl = (ConfigDepotImpl)ReflectionTestUtils.getField(configKey, "s_depot");
         ConfigDepotImpl configDepot = Mockito.mock(ConfigDepotImpl.class);
-        ScopedConfigStorage storage = Mockito.mock(ScopedConfigStorage.class);
-        Mockito.when(storage.getConfigValue(Mockito.anyLong(), Mockito.eq(configKey))).thenReturn(manufacturer);
-        Mockito.when(storage.getConfigValue(Mockito.anyLong(), Mockito.eq(VirtualMachineManager.VmMetadataProductName)))
-                .thenReturn(product);
-        Mockito.when(configDepot.findScopedConfigStorage(configKey)).thenReturn(storage);
-        Mockito.when(configDepot.findScopedConfigStorage(VirtualMachineManager.VmMetadataProductName)).thenReturn(storage);
+        Mockito.when(configDepot.getConfigStringValue(Mockito.eq(configKey.key()),
+                Mockito.eq(ConfigKey.Scope.Zone), Mockito.anyLong())).thenReturn(manufacturer);
+        Mockito.when(configDepot.getConfigStringValue(Mockito.eq(VirtualMachineManager.VmMetadataProductName.key()),
+                Mockito.eq(ConfigKey.Scope.Zone), Mockito.anyLong())).thenReturn(product);
         ReflectionTestUtils.setField(configKey, "s_depot", configDepot);
         updatedConfigKeyDepot = true;
     }

diff --git a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.Map;
 
-
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.ConfigKey.Scope;
 import org.apache.cloudstack.framework.config.ScopedConfigStorage;
@@ -136,8 +135,8 @@ public Scope getScope() {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
-        ClusterDetailsVO vo = findDetail(id, key.key());
+    public String getConfigValue(long id, String key) {
+        ClusterDetailsVO vo = findDetail(id, key);
         return vo == null ? null : vo.getValue();
     }
 

diff --git a/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDetailsDaoImpl.java
@@ -44,8 +44,8 @@ public Scope getScope() {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
-        ResourceDetail vo = findDetail(id, key.key());
+    public String getConfigValue(long id, String key) {
+        ResourceDetail vo = findDetail(id, key);
         return vo == null ? null : vo.getValue();
     }
 

diff --git a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java
@@ -22,6 +22,11 @@
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.framework.config.ConfigKey.Scope;
+import org.apache.cloudstack.framework.config.ScopedConfigStorage;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
+
 import com.cloud.domain.DomainDetailVO;
 import com.cloud.domain.DomainVO;
 import com.cloud.utils.crypt.DBEncryptionUtil;
@@ -31,11 +36,6 @@
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
-import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.ConfigKey.Scope;
-import org.apache.cloudstack.framework.config.ScopedConfigStorage;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 
 public class DomainDetailsDaoImpl extends GenericDaoBase<DomainDetailVO, Long> implements DomainDetailsDao, ScopedConfigStorage {
     protected final SearchBuilder<DomainDetailVO> domainSearch;
@@ -108,17 +108,17 @@ public Scope getScope() {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
+    public String getConfigValue(long id, String key) {
         DomainDetailVO vo = null;
         String enableDomainSettingsForChildDomain = _configDao.getValue("enable.domain.settings.for.child.domain");
         if (!Boolean.parseBoolean(enableDomainSettingsForChildDomain)) {
-            vo = findDetail(id, key.key());
+            vo = findDetail(id, key);
             return vo == null ? null : getActualValue(vo);
         }
         DomainVO domain = _domainDao.findById(id);
         // if value is not configured in domain then check its parent domain till ROOT
         while (domain != null) {
-            vo = findDetail(domain.getId(), key.key());
+            vo = findDetail(domain.getId(), key);
             if (vo != null) {
                 break;
             } else if (domain.getParent() != null) {

diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java
@@ -17,6 +17,10 @@
 package com.cloud.storage.dao;
 
 
+import java.util.List;
+
+import javax.inject.Inject;
+
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.ConfigKey.Scope;
 import org.apache.cloudstack.framework.config.ScopedConfigStorage;
@@ -26,9 +30,6 @@
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 
-import javax.inject.Inject;
-import java.util.List;
-
 public class StoragePoolDetailsDaoImpl extends ResourceDetailsDaoBase<StoragePoolDetailVO> implements StoragePoolDetailsDao, ScopedConfigStorage {
 
     @Inject
@@ -43,8 +44,8 @@ public Scope getScope() {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
-        StoragePoolDetailVO vo = findDetail(id, key.key());
+    public String getConfigValue(long id, String key) {
+        StoragePoolDetailVO vo = findDetail(id, key);
         return vo == null ? null : vo.getValue();
     }
 

diff --git a/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java
@@ -23,25 +23,24 @@
 
 import javax.inject.Inject;
 
-import com.cloud.utils.crypt.DBEncryptionUtil;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.ConfigKey.Scope;
 import org.apache.cloudstack.framework.config.ScopedConfigStorage;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 
 import com.cloud.domain.DomainDetailVO;
 import com.cloud.domain.DomainVO;
-import com.cloud.domain.dao.DomainDetailsDao;
 import com.cloud.domain.dao.DomainDao;
+import com.cloud.domain.dao.DomainDetailsDao;
 import com.cloud.user.dao.AccountDao;
-
+import com.cloud.utils.crypt.DBEncryptionUtil;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.QueryBuilder;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 
 public class AccountDetailsDaoImpl extends GenericDaoBase<AccountDetailVO, Long> implements AccountDetailsDao, ScopedConfigStorage {
     protected final SearchBuilder<AccountDetailVO> accountSearch;
@@ -118,9 +117,9 @@ public Scope getScope() {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
+    public String getConfigValue(long id, String key) {
         // check if account level setting is configured
-        AccountDetailVO vo = findDetail(id, key.key());
+        AccountDetailVO vo = findDetail(id, key);
         String value = vo == null ? null : getActualValue(vo);
         if (value != null) {
             return value;
@@ -140,7 +139,7 @@ public String getConfigValue(long id, ConfigKey<?> key) {
             if (account.isPresent()) {
                 DomainVO domain = _domainDao.findById(account.get().getDomainId());
                 while (domain != null) {
-                    DomainDetailVO domainVO = _domainDetailsDao.findDetail(domain.getId(), key.key());
+                    DomainDetailVO domainVO = _domainDetailsDao.findDetail(domain.getId(), key);
                     if (domainVO != null) {
                         value = _domainDetailsDao.getActualValue(domainVO);
                         break;

diff --git a/...ma/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java b/...ma/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java
@@ -20,6 +20,11 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.ConfigKey.Scope;
+import org.apache.cloudstack.framework.config.ScopedConfigStorage;
+import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
 import org.springframework.stereotype.Component;
 
 import com.cloud.utils.crypt.DBEncryptionUtil;
@@ -29,12 +34,6 @@
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
 
-import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.ConfigKey.Scope;
-import org.apache.cloudstack.framework.config.ScopedConfigStorage;
-import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
-
 @Component
 public class ImageStoreDetailsDaoImpl extends ResourceDetailsDaoBase<ImageStoreDetailVO> implements ImageStoreDetailsDao, ScopedConfigStorage {
 
@@ -106,8 +105,8 @@ public ImageStoreDetailVO findDetail(long storeId, String name) {
     }
 
     @Override
-    public String getConfigValue(long id, ConfigKey<?> key) {
-        ImageStoreDetailVO vo = findDetail(id, key.key());
+    public String getConfigValue(long id, String key) {
+        ImageStoreDetailVO vo = findDetail(id, key);
         return vo == null ? null : vo.getValue();
     }
 

diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigDepot.java
@@ -32,4 +32,6 @@ public interface ConfigDepot {
 
     <T> void createOrUpdateConfigObject(String componentName, ConfigKey<T> key, String value);
     boolean isNewConfig(ConfigKey<?> configKey);
+    String getConfigStringValue(String key, ConfigKey.Scope scope, Long scopeId);
+    void invalidateConfigCache(String key, ConfigKey.Scope scope, Long scopeId);
 }
diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
@@ -19,7 +19,6 @@
 import java.sql.Date;
 
 import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl;
-import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
@@ -215,42 +214,38 @@ public boolean isSameKeyAs(Object obj) {
 
     public T value() {
         if (_value == null || isDynamic()) {
-            ConfigurationVO vo = (s_depot != null && s_depot.global() != null) ? s_depot.global().findById(key()) : null;
-            final String value = (vo != null && vo.getValue() != null) ? vo.getValue() : defaultValue();
-            _value = ((value == null) ? (T)defaultValue() : valueOf(value));
+            String value = s_depot != null ? s_depot.getConfigStringValue(_name, Scope.Global, null) : null;
+            _value = valueOf((value == null) ? defaultValue() : value);
         }
 
         return _value;
     }
 
-    public T valueIn(Long id) {
+    protected T valueInScope(Scope scope, Long id) {
         if (id == null) {
             return value();
         }
 
-        String value = s_depot != null ? s_depot.findScopedConfigStorage(this).getConfigValue(id, this) : null;
+        String value = s_depot != null ? s_depot.getConfigStringValue(_name, scope, id) : null;
         if (value == null) {
             return value();
-        } else {
-            return valueOf(value);
         }
+        return valueOf(value);
     }
 
-    public T valueInDomain(Long domainId) {
-        if (domainId == null) {
-            return value();
-        }
+    public T valueIn(Long id) {
+        return valueInScope(_scope, id);
+    }
 
-        String value = s_depot != null ? s_depot.getDomainScope(this).getConfigValue(domainId, this) : null;
-        if (value == null) {
-            return value();
-        } else {
-            return valueOf(value);
-        }
+    public T valueInDomain(Long domainId) {
+        return valueInScope(Scope.Domain, domainId);
     }
 
     @SuppressWarnings("unchecked")
     protected T valueOf(String value) {
+        if (value == null) {
+            return null;
+        }
         Number multiplier = 1;
         if (multiplier() != null) {
             multiplier = (Number)multiplier();

diff --git a/...work/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java b/...work/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java
@@ -26,5 +26,9 @@
 public interface ScopedConfigStorage {
     Scope getScope();
 
-    String getConfigValue(long id, ConfigKey<?> key);
+    String getConfigValue(long id, String key);
+
+    default String getConfigValue(long id, ConfigKey<?> key) {
+        return getConfigValue(id, key.key());
+    }
 }
diff --git a/...ork/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java b/...ork/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
@@ -23,6 +23,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
@@ -37,12 +38,14 @@
 import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDao;
 import org.apache.commons.lang.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
 
 /**
  * ConfigDepotImpl implements the ConfigDepot and ConfigDepotAdmin interface.
@@ -73,6 +76,7 @@
  */
 public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
     protected Logger logger = LogManager.getLogger(getClass());
+    protected final static long CONFIG_CACHE_EXPIRE_SECONDS = 30;
     @Inject
     ConfigurationDao _configDao;
     @Inject
@@ -83,12 +87,17 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
     List<ScopedConfigStorage> _scopedStorages;
     Set<Configurable> _configured = Collections.synchronizedSet(new HashSet<Configurable>());
     Set<String> newConfigs = Collections.synchronizedSet(new HashSet<>());
+    Cache<String, String> configCache;
 
     private HashMap<String, Pair<String, ConfigKey<?>>> _allKeys = new HashMap<String, Pair<String, ConfigKey<?>>>(1007);
 
     HashMap<ConfigKey.Scope, Set<ConfigKey<?>>> _scopeLevelConfigsMap = new HashMap<ConfigKey.Scope, Set<ConfigKey<?>>>();
 
     public ConfigDepotImpl() {
+        configCache = Caffeine.newBuilder()
+                .maximumSize(512)
+                .expireAfterWrite(CONFIG_CACHE_EXPIRE_SECONDS, TimeUnit.SECONDS)
+                .build();
         ConfigKey.init(this);
         createEmptyScopeLevelMappings();
     }
@@ -268,6 +277,48 @@ public ConfigurationDao global() {
         return _configDao;
     }
 
+    protected String getConfigStringValueInternal(String cacheKey) {
+        String[] parts = cacheKey.split("-");
+        String key = parts[0];
+        ConfigKey.Scope scope = ConfigKey.Scope.Global;
+        Long scopeId = null;
+        try {
+            scope = ConfigKey.Scope.valueOf(parts[1]);
+            scopeId = Long.valueOf(parts[2]);
+        } catch (IllegalArgumentException ignored) {}
+        if (!ConfigKey.Scope.Global.equals(scope) && scopeId != null) {
+            ScopedConfigStorage scopedConfigStorage = null;
+            for (ScopedConfigStorage storage : _scopedStorages) {
+                if (storage.getScope() == scope) {
+                    scopedConfigStorage = storage;
+                }
+            }
+            if (scopedConfigStorage == null) {
+                throw new CloudRuntimeException("Unable to find config storage for this scope: " + scope + " for " + key);
+            }
+            return scopedConfigStorage.getConfigValue(scopeId, key);
+        }
+        ConfigurationVO configurationVO = _configDao.findById(key);
+        if (configurationVO != null) {
+            return configurationVO.getValue();
+        }
+        return null;
+    }
+
+    private String getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) {
+        return String.format("%s-%s-%d", key, scope, (scopeId == null ? 0 : scopeId));
+    }
+
+    @Override
+    public String getConfigStringValue(String key, ConfigKey.Scope scope, Long scopeId) {
+        return configCache.get(getConfigCacheKey(key, scope, scopeId), this::getConfigStringValueInternal);
+    }
+
+    @Override
+    public void invalidateConfigCache(String key, ConfigKey.Scope scope, Long scopeId) {
+        configCache.invalidate(getConfigCacheKey(key, scope, scopeId));
+    }
+
     public ScopedConfigStorage findScopedConfigStorage(ConfigKey<?> config) {
         for (ScopedConfigStorage storage : _scopedStorages) {
             if (storage.getScope() == config.scope()) {