diff --git a/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java b/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java index 21eb5af4..a7bff78f 100644 --- a/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java +++ b/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java @@ -17,7 +17,6 @@ */ package com.netflix.client.config; -import com.netflix.config.ConfigurationManager; import java.util.concurrent.TimeUnit; @@ -302,7 +301,6 @@ public DefaultClientConfigImpl(String nameSpace) { } public void loadDefaultValues() { - super.loadDefaultValues(); set(CommonClientConfigKey.MaxHttpConnectionsPerHost, getDefaultMaxHttpConnectionsPerHost()); set(CommonClientConfigKey.MaxTotalHttpConnections, getDefaultMaxTotalHttpConnections()); set(CommonClientConfigKey.EnableConnectionPool, getDefaultEnableConnectionPool()); @@ -319,18 +317,8 @@ public void loadDefaultValues() { set(CommonClientConfigKey.ConnIdleEvictTimeMilliSeconds, getDefaultConnectionidleTimeInMsecs()); set(CommonClientConfigKey.ConnectionCleanerRepeatInterval, getDefaultConnectionIdleTimertaskRepeatInMsecs()); set(CommonClientConfigKey.EnableGZIPContentEncodingFilter, getDefaultEnableGzipContentEncodingFilter()); - String proxyHost = ConfigurationManager.getConfigInstance().getString(getDefaultPropName(CommonClientConfigKey.ProxyHost.key())); - if (proxyHost != null && proxyHost.length() > 0) { - set(CommonClientConfigKey.ProxyHost, proxyHost); - } - Integer proxyPort = ConfigurationManager - .getConfigInstance() - .getInteger( - getDefaultPropName(CommonClientConfigKey.ProxyPort), - (Integer.MIN_VALUE + 1)); // + 1 just to avoid potential clash with user setting - if (proxyPort != (Integer.MIN_VALUE + 1)) { - set(CommonClientConfigKey.ProxyPort, proxyPort); - } + set(CommonClientConfigKey.ProxyHost, null); + set(CommonClientConfigKey.ProxyPort, null); set(CommonClientConfigKey.Port, getDefaultPort()); set(CommonClientConfigKey.EnablePrimeConnections, getDefaultEnablePrimeConnections()); set(CommonClientConfigKey.MaxRetriesPerServerPrimeConnection, getDefaultMaxRetriesPerServerPrimeConnection()); diff --git a/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java b/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java index 8e272006..6c7bc18f 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java @@ -24,6 +24,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.HashSet; +import java.util.Objects; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -285,4 +286,16 @@ public String toString() { @Override public T defaultValue() { return defaultValue; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CommonClientConfigKey that = (CommonClientConfigKey) o; + return Objects.equals(configKey, that.configKey); + } + + @Override + public int hashCode() { + return Objects.hash(configKey); + } } diff --git a/ribbon-core/src/main/java/com/netflix/client/config/IClientConfig.java b/ribbon-core/src/main/java/com/netflix/client/config/IClientConfig.java index c342a6dc..36e69f50 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/IClientConfig.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/IClientConfig.java @@ -49,14 +49,6 @@ public interface IClientConfig { Map getProperties(); - /** - * Iterate all properties and report the default value only to the consumer - * @param consumer - */ - default void forEachDefault(BiConsumer, Object> consumer) { - throw new UnsupportedOperationException(); - } - /** * Iterate all properties and report the final value. Can be null if a default value is not specified. * @param consumer diff --git a/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java b/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java index bcee7135..470b38fb 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java @@ -39,7 +39,7 @@ public abstract class ReloadableClientConfig implements IClientConfig { private static final String DEFAULT_NAMESPACE = "ribbon"; // Map of raw property names (without namespace or client) to values set via code - private final Map internalProperties = new HashMap<>(); + private final Map internalProperties = new HashMap<>(); // Map of all seen dynamic properties. This map will hold on properties requested with the exception of // those returned from getGlobalProperty(). @@ -57,19 +57,15 @@ public abstract class ReloadableClientConfig implements IClientConfig { private String namespace = DEFAULT_NAMESPACE; - private boolean isDynamic; - private boolean isLoaded = false; protected ReloadableClientConfig(PropertyResolver resolver) { this.clientName = DEFAULT_CLIENT_NAME; - this.isDynamic = false; this.resolver = resolver; } protected ReloadableClientConfig(PropertyResolver resolver, String clientName) { this.clientName = clientName; - this.isDynamic = true; this.resolver = resolver; } @@ -112,29 +108,24 @@ public final void setNameSpace(String nameSpace) { public void loadProperties(String clientName) { Preconditions.checkState(isLoaded == false, "Config '{}' can only be loaded once", clientName); if (!isLoaded) { + loadDefaultValues(); this.isLoaded = true; resolver.onChange(this::reload); } - this.isDynamic = true; this.clientName = clientName; - loadDefaultValues(); - } - - @Override - public void loadDefaultValues() { - this.isDynamic = true; - reload(); } @Override public final Map getProperties() { - return Collections.unmodifiableMap(internalProperties); - } - - @Override - public void forEachDefault(BiConsumer, Object> consumer) { - dynamicProperties.forEach((key, value) -> consumer.accept(key, internalProperties.get(key.key()))); + final Map result = new HashMap<>(dynamicProperties.size()); + dynamicProperties.forEach((key, value) -> { + Object v = value.get().orElse(null); + if (v != null) { + result.put(key.key(), String.valueOf(v)); + } + }); + return result; } @Override @@ -150,9 +141,7 @@ private ReloadableProperty createProperty(final Supplier> val { refresh(); - if (isDynamic) { - changeActions.add(this::refresh); - } + changeActions.add(this::refresh); } @Override @@ -192,11 +181,7 @@ public String toString() { @Override public final T get(IClientConfigKey key) { - if (isLoaded) { - return getOrCreateProperty(key).get().orElse(null); - } else { - return getIfSet(key).orElse(null); - } + return getOrCreateProperty(key).get().orElse(null); } private final ReloadableProperty getOrCreateProperty(IClientConfigKey key) { @@ -251,6 +236,12 @@ private Optional resolveFinalProperty(IClientConfigKey key) { return getIfSet(key); } + /** + * Resolve a p + * @param key + * @param + * @return + */ private Optional resolverScopedProperty(IClientConfigKey key) { Optional value = resolver.get(clientName + "." + getNameSpace() + "." + key.key(), key.type()); if (value.isPresent()) { @@ -262,7 +253,7 @@ private Optional resolverScopedProperty(IClientConfigKey key) { @Override public Optional getIfSet(IClientConfigKey key) { - return Optional.ofNullable(internalProperties.get(key.key())) + return Optional.ofNullable(internalProperties.get(key)) .map(value -> { final Class type = key.type(); // Unfortunately there's some legacy code setting string values for typed keys. Here are do our best to parse @@ -359,7 +350,7 @@ public final IClientConfig set(IClientConfigKey key, T value) { if (value == null) { internalProperties.remove(key.key()); } else { - internalProperties.put(key.key(), value); + internalProperties.put(key, value); } if (isLoaded) { @@ -418,8 +409,16 @@ public IClientConfig applyOverride(IClientConfig override) { return this; } - this.internalProperties.putAll(override.getProperties()); - reload(); + // When overriding we only really care of picking up properties that were explicitly set in code. This is a + // bit of an optimization to avoid excessive memory allocation as requests are made and overrides are applied + if (override instanceof ReloadableClientConfig) { + ((ReloadableClientConfig)override).internalProperties.forEach((key, value) -> { + if (value != null) { + setProperty(key, value); + } + }); + } + return this; } diff --git a/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java b/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java index b1c79e00..2cf35432 100644 --- a/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java +++ b/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java @@ -30,6 +30,8 @@ import org.junit.Test; import org.junit.rules.TestName; import org.junit.runners.MethodSorters; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Properties; @@ -41,6 +43,8 @@ */ @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class ClientConfigTest { + private static final Logger LOG = LoggerFactory.getLogger(ClientConfigTest.class); + @Rule public TestName testName = new TestName(); @@ -186,7 +190,7 @@ public void testValueOf() { ConfigurationManager.getConfigInstance().setProperty("testValueOf.ribbon.CustomValueOf", "value"); DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl(); - clientConfig.setClientName("testValueOf"); + clientConfig.loadProperties("testValueOf"); Property prop = clientConfig.getDynamicProperty(CUSTOM_KEY); Assert.assertEquals("value", prop.getOrDefault().getValue()); @@ -198,7 +202,7 @@ public void testScopedProperty() { ConfigurationManager.getConfigInstance().setProperty("testScopedProperty.ribbon.foo.ScopePropertyTimeout", "1000"); DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl(); - clientConfig.setClientName("testScopedProperty"); + clientConfig.loadProperties("testScopedProperty"); Property prop = clientConfig.getScopedProperty(new CommonClientConfigKey("foo.ScopePropertyTimeout", 0) {}); Assert.assertEquals(1000, prop.get().get().intValue()); diff --git a/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/SubsetFilterTest.java b/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/SubsetFilterTest.java index d46fa03e..9524e513 100644 --- a/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/SubsetFilterTest.java +++ b/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/SubsetFilterTest.java @@ -84,7 +84,7 @@ public void testSorting() { @Test public void testFiltering() { DefaultClientConfigImpl config = new DefaultClientConfigImpl(); - config.setClientName("SubsetFilerTest"); + config.loadProperties("SubsetFilerTest"); ServerListSubsetFilter filter = new ServerListSubsetFilter(config); LoadBalancerStats stats = new LoadBalancerStats("default"); diff --git a/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java b/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java index 58fe6a8b..1fd8e882 100644 --- a/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java +++ b/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java @@ -74,7 +74,7 @@ public void testChooseZone() throws Exception { ConfigurationManager.getConfigInstance().setProperty("niws.loadbalancer.serverStats.activeRequestsCount.effectiveWindowSeconds", 10); DefaultClientConfigImpl config = new DefaultClientConfigImpl(); - config.setClientName("testChooseZone"); + config.loadProperties("testChooseZone"); ZoneAwareLoadBalancer balancer = new ZoneAwareLoadBalancer(); balancer.init(); IRule globalRule = new RoundRobinRule();