Skip to content

Configuration keys performance update #14173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sb-abhish3k wants to merge 1 commit into
base: 4.22
Choose a base branch
from
Draft

Configuration keys performance update #14173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,15 @@ public T valueInScope(Scope scope, Long id) {
}
String value = s_depot != null ? s_depot.getConfigStringValue(_name, scope, id) : null;
if (value == null) {
return valueInGlobalOrAvailableParentScope(scope, id);
T parentValue = valueInGlobalOrAvailableParentScope(scope, id);
// Cache the inherited value under this scope to avoid repeated hierarchy traversal.
// Skip this for keys with a multiplier: parentValue already has the multiplier applied,
// so caching its toString() and reading it back through valueOf() would apply the
// multiplier a second time (double scaling).
if (s_depot != null && parentValue != null && multiplier() == null) {
s_depot.cacheValue(_name, scope, id, parentValue.toString());
}
return parentValue;
}
logger.trace("Scope({}) value for config ({}): {}", scope, _name, _value);

Expand Down
2 changes: 2 additions & 0 deletions ...ork/config/src/main/java/org/apache/cloudstack/framework/config/dao/ConfigurationDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,6 @@ public interface ConfigurationDao extends GenericDao<ConfigurationVO, String> {
void invalidateCache();

List<ConfigurationVO> searchPartialConfigurations();

String getValueByKey(String key);
}
6 changes: 6 additions & 0 deletions ...config/src/main/java/org/apache/cloudstack/framework/config/dao/ConfigurationDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,10 @@ public List<ConfigurationVO> searchPartialConfigurations() {
SearchCriteria<ConfigurationVO> sc = PartialSearch.create();
return searchIncludingRemoved(sc, null, null, false);
}

@Override
public String getValueByKey(String key) {
ConfigurationVO configVO = findByName(key);
return (configVO == null ? null : configVO.getValue());
}
}
107 changes: 94 additions & 13 deletions ...ork/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@
import java.util.List;
import java.util.Set;

import javax.annotation.PostConstruct;
import javax.inject.Inject;

import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.ConfigDepotAdmin;
import org.apache.cloudstack.framework.config.ConfigKey;
Expand All @@ -38,13 +35,16 @@
import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDao;
import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDao;
import org.apache.cloudstack.utils.cache.LazyCache;
import org.apache.commons.beanutils.ConvertUtils;
import org.apache.commons.lang.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
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.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.exception.CloudRuntimeException;

/**
Expand Down Expand Up @@ -74,9 +74,18 @@
* when constructing a ConfigKey then configuration server should use the
* validation class to validate the value the admin input for the key.
*/
public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin, Configurable {
protected Logger logger = LogManager.getLogger(getClass());

protected final static long CONFIG_CACHE_EXPIRE_SECONDS = 30;

protected final ConfigKey<Long> ConfigKeyCacheMaxSize = new ConfigKey<>("Advanced", Long.class, "config.key.cache.max.size", "512",
"Configuration keys cache max size", false);
protected final ConfigKey<Long> ConfigKeyCacheRefreshIntervalSeconds = new ConfigKey<>("Advanced", Long.class, "config.key.expire.seconds", String.valueOf(CONFIG_CACHE_EXPIRE_SECONDS),
"Configuration keys cache refresh interval in seconds", false);
protected final ConfigKey<Boolean> ConfigKeyCacheRefreshAfterWrite = new ConfigKey<>("Advanced", Boolean.class, "config.key.cache.refresh.after.write", "false",
"When true the configuration cache refreshes entries asynchronously and serves the stale value during reload (non-blocking); when false entries expire and the next read blocks to load a fresh value (stronger consistency across management servers)", false);

@Inject
ConfigurationDao _configDao;
@Inject
Expand All @@ -87,15 +96,13 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
List<ScopedConfigStorage> _scopedStorages;
Set<Configurable> _configured = Collections.synchronizedSet(new HashSet<Configurable>());
Set<String> newConfigs = Collections.synchronizedSet(new HashSet<>());
LazyCache<Ternary<String, ConfigKey.Scope, Long>, String> configCache;
volatile LazyCache<Ternary<String, ConfigKey.Scope, Long>, 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 = new LazyCache<>(512,
CONFIG_CACHE_EXPIRE_SECONDS, this::getConfigStringValueInternal);
ConfigKey.init(this);
createEmptyScopeLevelMappings();
}
Expand All @@ -121,7 +128,63 @@ public ConfigKey<?> get(String key) {
return value != null ? value.second() : null;
}

@PostConstruct
@SuppressWarnings("unchecked")
private <T> T getConfigValue(ConfigKey<T> configKey) {
String valueString;
try {
valueString = _configDao.getValueByKey(configKey.key());
if (valueString == null) {
valueString = configKey.defaultValue();
}
} catch (CloudRuntimeException e) {
String msg = "Failed to retrieve configuration value for: " + configKey.key();
logger.error(msg, e);
throw e;
} catch (Exception e) {
String msg = "Failed to retrieve configuration value for: " + configKey.key();
logger.error(msg, e);
throw new CloudRuntimeException(msg, e);
}
return (T) ConvertUtils.convert(valueString, configKey.type());
}

/**
* Lazily initialize the config cache on first access. Reading the cache size and
* TTL here is safe because by the time any caller exercises the cache, Spring's
* refresh() has completed and DatabaseUpgradeChecker has run any pending schema
* migrations, so the configuration table is in its expected shape.
*
* populateConfiguration(this) is invoked here to guarantee that this bean's own
* ConfigKeys end up registered in _allKeys and persisted to the configuration
* table even when Spring's List<Configurable> autowiring excludes self. The call
* is idempotent: if ConfigurationServerImpl.populateConfigurations() already
* iterated over this bean, the _configured guard inside populateConfiguration
* makes it a no-op.
*
* The cache-tuning keys are read directly from the configuration table via
* getConfigValue(); they fall back to their defaults when no row is present. They are
* applied only here at initialization (the cache is built once), so changing them
* requires a restart.
*/
private void ensureCacheInitialized() {
if (configCache == null) {
synchronized (this) {
if (configCache == null) {
populateConfiguration(this);
Long maxSize = getConfigValue(ConfigKeyCacheMaxSize);
Long expirationSeconds = getConfigValue(ConfigKeyCacheRefreshIntervalSeconds);
Boolean refreshAfterWrite = getConfigValue(ConfigKeyCacheRefreshAfterWrite);
if (logger.isDebugEnabled()) {
logger.debug("{} value: {}", ConfigKeyCacheMaxSize.key(), maxSize);
logger.debug("{} value: {}", ConfigKeyCacheRefreshIntervalSeconds.key(), expirationSeconds);
logger.debug("{} value: {}", ConfigKeyCacheRefreshAfterWrite.key(), refreshAfterWrite);
}
configCache = new LazyCache<>(maxSize, expirationSeconds, refreshAfterWrite, this::getConfigStringValueInternal);
}
}
}
}

@Override
public void populateConfigurations() {
Date date = new Date();
Expand Down Expand Up @@ -282,6 +345,7 @@ protected String getConfigStringValueInternal(Ternary<String, ConfigKey.Scope, L
final String key = cacheKey.first();
final ConfigKey.Scope scope = cacheKey.second();
final Long scopeId = cacheKey.third();
logger.debug("Fetching config key from DB: key={}, scope={}, scopeId={}", key, scope, scopeId);
if (!ConfigKey.Scope.Global.equals(scope) && scopeId != null) {
ScopedConfigStorage scopedConfigStorage = getScopedStorage(scope);
if (scopedConfigStorage == null) {
Expand All @@ -290,11 +354,7 @@ protected String getConfigStringValueInternal(Ternary<String, ConfigKey.Scope, L
final ScopedConfigStorage scopedConfigStorageFinal = scopedConfigStorage;
return Transaction.execute((TransactionCallback<String>) status -> scopedConfigStorageFinal.getConfigValue(scopeId, key));
}
ConfigurationVO configurationVO = _configDao.findById(key);
if (configurationVO != null) {
return configurationVO.getValue();
}
return null;
return _configDao.getValueByKey(key);
}

protected Ternary<String, ConfigKey.Scope, Long> getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) {
Expand All @@ -303,11 +363,22 @@ protected Ternary<String, ConfigKey.Scope, Long> getConfigCacheKey(String key, C

@Override
public String getConfigStringValue(String key, ConfigKey.Scope scope, Long scopeId) {
ensureCacheInitialized();
return configCache.get(getConfigCacheKey(key, scope, scopeId));
}

/**
* Inserts a value directly into the config cache without persisting to DB.
* Used to cache inherited values (e.g. from a parent scope) under a more specific scope key.
*/
public void cacheValue(String key, ConfigKey.Scope scope, Long scopeId, String value) {
ensureCacheInitialized();
configCache.put(getConfigCacheKey(key, scope, scopeId), value);
}

@Override
public void invalidateConfigCache(String key, ConfigKey.Scope scope, Long scopeId) {
ensureCacheInitialized();
configCache.invalidate(getConfigCacheKey(key, scope, scopeId));
}

Expand Down Expand Up @@ -397,4 +468,14 @@ public Pair<ConfigKey.Scope, Long> getParentScope(ConfigKey.Scope scope, Long id
}
return scopedConfigStorage.getParentScope(id);
}

@Override
public String getConfigComponentName() {
return ConfigDepotImpl.class.getSimpleName();
}

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey[]{ConfigKeyCacheMaxSize, ConfigKeyCacheRefreshIntervalSeconds, ConfigKeyCacheRefreshAfterWrite};
}
}
4 changes: 2 additions & 2 deletions ...onfig/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotAdminTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ public void setUp() throws Exception {
_depotAdmin._configDao = _configDao;
_depotAdmin._configGroupDao = _configGroupDao;
_depotAdmin._configSubGroupDao = _configSubGroupDao;
_depotAdmin._configurables = new ArrayList<Configurable>();
_depotAdmin._configurables = new ArrayList<>();
_depotAdmin._configurables.add(_configurable);
_depotAdmin._scopedStorages = new ArrayList<ScopedConfigStorage>();
_depotAdmin._scopedStorages = new ArrayList<>();
_depotAdmin._scopedStorages.add(_scopedStorage);
}

Expand Down
98 changes: 94 additions & 4 deletions ...config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDao;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
Expand All @@ -41,6 +42,7 @@

import com.cloud.utils.Pair;


@RunWith(MockitoJUnitRunner.class)
public class ConfigDepotImplTest {

Expand All @@ -55,6 +57,12 @@ public class ConfigDepotImplTest {
@InjectMocks
private ConfigDepotImpl configDepotImpl = new ConfigDepotImpl();

@Before
public void setUp() {
configDepotImpl.setConfigurables(Collections.emptyList());
configDepotImpl.populateConfigurations();
}

@Test
public void createConfigObjectPersistsSubGroupWithNameAndGroupId() {
ConfigKey<?> key = Mockito.mock(ConfigKey.class);
Expand Down Expand Up @@ -102,9 +110,7 @@ public void testIsNewConfig() {
}

private void runTestGetConfigStringValue(String key, String value) {
ConfigurationVO configurationVO = Mockito.mock(ConfigurationVO.class);
Mockito.when(configurationVO.getValue()).thenReturn(value);
Mockito.when(_configDao.findById(key)).thenReturn(configurationVO);
Mockito.when(_configDao.getValueByKey(key)).thenReturn(value);
String result = configDepotImpl.getConfigStringValue(key, ConfigKey.Scope.Global, null);
Assert.assertEquals(value, result);
}
Expand All @@ -131,7 +137,7 @@ private void runTestGetConfigStringValueExpiry(long wait, int configDBRetrieval)
}
String result = configDepotImpl.getConfigStringValue(key, ConfigKey.Scope.Global, null);
Assert.assertEquals(value, result);
Mockito.verify(_configDao, Mockito.times(configDBRetrieval)).findById(key);
Mockito.verify(_configDao, Mockito.timeout(2000).times(configDBRetrieval)).getValueByKey(key);
}

@Test
Expand Down Expand Up @@ -199,6 +205,58 @@ public ConfigKey<?>[] getConfigKeys() {
Mockito.verify(_configDao, Mockito.times(1)).persist(configurationVO);
}

@Test
public void testParentScopeValueCachedUnderChildScopeOnMiss() {
String keyName = "test.key";
ConfigKey<String> key = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class,
keyName, "default-value", "test", true, List.of(ConfigKey.Scope.Cluster, ConfigKey.Scope.Zone));

Long clusterId = 1L;
Long zoneId = 2L;
String zoneValue = "zone-value";

ScopedConfigStorage clusterStorage = Mockito.mock(ScopedConfigStorage.class);
Mockito.when(clusterStorage.getScope()).thenReturn(ConfigKey.Scope.Cluster);
Mockito.when(clusterStorage.getConfigValue(clusterId, keyName)).thenReturn(null);
Mockito.when(clusterStorage.getParentScope(clusterId)).thenReturn(new Pair<>(ConfigKey.Scope.Zone, zoneId));

ScopedConfigStorage zoneStorage = Mockito.mock(ScopedConfigStorage.class);
Mockito.when(zoneStorage.getScope()).thenReturn(ConfigKey.Scope.Zone);
Mockito.when(zoneStorage.getConfigValue(zoneId, keyName)).thenReturn(zoneValue);

configDepotImpl.setScopedStorages(List.of(clusterStorage, zoneStorage));

// first call: no cluster value, traverses to zone, zone value is cached under cluster scope key
Assert.assertEquals(zoneValue, key.valueInScope(ConfigKey.Scope.Cluster, clusterId));

// second call: cache hit with zone value, cluster storage not queried again
Assert.assertEquals(zoneValue, key.valueInScope(ConfigKey.Scope.Cluster, clusterId));
Mockito.verify(clusterStorage, Mockito.times(1)).getConfigValue(clusterId, keyName);
}

@Test
public void testDefaultValueCachedUnderChildScopeOnMiss() {
String keyName = "test.key";
String keyValue = "default-value";
ConfigKey<String> key = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class,
keyName, keyValue, "test", true, ConfigKey.Scope.Cluster);

Long clusterId = 1L;

ScopedConfigStorage clusterStorage = Mockito.mock(ScopedConfigStorage.class);
Mockito.when(clusterStorage.getScope()).thenReturn(ConfigKey.Scope.Cluster);
Mockito.when(clusterStorage.getConfigValue(clusterId, keyName)).thenReturn(null);

configDepotImpl.setScopedStorages(List.of(clusterStorage));

// first call: no cluster value, traverses to default, default value is cached under cluster scope key
Assert.assertEquals(keyValue, key.valueInScope(ConfigKey.Scope.Cluster, clusterId));

// second call: cache hit with default value, cluster storage not queried again
Assert.assertEquals(keyValue, key.valueInScope(ConfigKey.Scope.Cluster, clusterId));
Mockito.verify(clusterStorage, Mockito.times(1)).getConfigValue(clusterId, keyName);
}

@Test
public void getParentScopeWithValidScope() {
ConfigKey.Scope scope = ConfigKey.Scope.Cluster;
Expand All @@ -217,4 +275,36 @@ public void getParentScopeWithValidScope() {
Assert.assertEquals(parentScope, result.first());
Assert.assertEquals(parentId, result.second());
}

@Test
public void testCacheNotInitializedBeforeFirstAccess() {
Assert.assertNull("configCache should be null until first access triggers lazy init",
configDepotImpl.configCache);
}

@Test
public void testCacheInitializedOnFirstAccess() {
Assert.assertNull(configDepotImpl.configCache);
configDepotImpl.getConfigStringValue("anyKey", ConfigKey.Scope.Global, null);
Assert.assertNotNull("configCache should be initialized after first access",
configDepotImpl.configCache);
}

@Test
public void testCacheInitializedOnlyOnce() {
configDepotImpl.getConfigStringValue("key1", ConfigKey.Scope.Global, null);
configDepotImpl.getConfigStringValue("key2", ConfigKey.Scope.Global, null);
configDepotImpl.getConfigStringValue("key3", ConfigKey.Scope.Global, null);
Mockito.verify(_configDao, Mockito.times(1)).getValueByKey("config.key.cache.max.size");
Mockito.verify(_configDao, Mockito.times(1)).getValueByKey("config.key.expire.seconds");
}

@Test
public void testCacheUsesDefaultsWhenConfigKeysAbsentInDB() {
Mockito.when(_configDao.getValueByKey("config.key.cache.max.size")).thenReturn(null);
Mockito.when(_configDao.getValueByKey("config.key.expire.seconds")).thenReturn(null);
configDepotImpl.getConfigStringValue("anyKey", ConfigKey.Scope.Global, null);
Assert.assertNotNull("Cache should initialize successfully with defaults when DB returns null",
configDepotImpl.configCache);
}
}
5 changes: 5 additions & 0 deletions server/src/test/java/com/cloud/vpc/dao/MockConfigurationDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,9 @@ public void invalidateCache() {
public List<ConfigurationVO> searchPartialConfigurations() {
return List.of();
}

@Override
public String getValueByKey(String key) {
return null;
}
}
Loading
Loading