Skip to content

Commit

Permalink
configuration: Decouple static archaius
Browse files Browse the repository at this point in the history
**Motivation**
Ribbon makes heavy use of Archaius in a static manner for configuration.  This pattern couple Ribbon with legacy archaius and its dependencies and makes it hard to run tests in parallel. By decoupling from both archaius and statics users can now provide alternative configuration mechanisms as well as improve testability.

**Changes**
- Introduce a new DynamicPropertyRepository abstraction to decouple from Archaius specific APIs
- For legacy support make the ArchaiusDynamicPropertyRepository discoverable via the service loader.
- Allow a non-static DynamicPropertyRepository to be associated with a IClientConfig.  This is a bit of a hack but minimizes code changes.
- Decouple all build.gradle dependencies from archaius, except for the ribbon-archaius subproject
- Pick up @zone from DynamicPropertyRepository and NOT from archaius's DeploymentContext
- Remove configuration for PollingServerListUpdater as it is not used and would always require static configuration
  • Loading branch information
elandau committed Mar 18, 2019
1 parent 375fbd1 commit 8d9d700
Show file tree
Hide file tree
Showing 53 changed files with 1,121 additions and 1,066 deletions.
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ guava_version=14.0.1
archaius_version=0.7.6
eureka_version=1.7.2
jersey_version=1.19.1
slf4j_version=1.7.2

junit_version=4.12
powermock_version=1.6.2
Expand Down
2 changes: 1 addition & 1 deletion ribbon-archaius/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
dependencies {
compile 'org.slf4j:slf4j-api:1.6.4'
compile "org.slf4j:slf4j-api:${slf4j_version}"
compile 'com.google.code.findbugs:annotations:2.0.0'
compile "com.google.guava:guava:${guava_version}"
compile 'commons-configuration:commons-configuration:1.8'
Expand Down

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion ribbon-core/build.gradle
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
dependencies {
compile 'org.slf4j:slf4j-api:1.6.4'
compile "org.slf4j:slf4j-api:${slf4j_version}"
compile 'com.google.code.findbugs:annotations:2.0.0'
compile "com.google.guava:guava:${guava_version}"
compile 'commons-lang:commons-lang:2.6'

testCompile 'junit:junit:4.11'
testCompile "org.slf4j:slf4j-log4j12:${slf4j_version}"
testCompile project(":ribbon-archaius")
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,19 @@ public abstract class CommonClientConfigKey<T> implements IClientConfigKey<T> {

public static final IClientConfigKey<Integer> SendBufferSize = new CommonClientConfigKey<Integer>("SendBufferSize"){};

public static final IClientConfigKey<Boolean> StaleCheckingEnabled = new CommonClientConfigKey<Boolean>("StaleCheckingEnabled"){};
public static final IClientConfigKey<Boolean> StaleCheckingEnabled = new CommonClientConfigKey<Boolean>("StaleCheckingEnabled", false){};

public static final IClientConfigKey<Integer> Linger = new CommonClientConfigKey<Integer>("Linger"){};
public static final IClientConfigKey<Integer> Linger = new CommonClientConfigKey<Integer>("Linger", 0){};

public static final IClientConfigKey<Integer> ConnectionManagerTimeout = new CommonClientConfigKey<Integer>("ConnectionManagerTimeout", 2000){};

public static final IClientConfigKey<Boolean> FollowRedirects = new CommonClientConfigKey<Boolean>("FollowRedirects", false){};

public static final IClientConfigKey<Boolean> ConnectionPoolCleanerTaskEnabled = new CommonClientConfigKey<Boolean>("ConnectionPoolCleanerTaskEnabled", true){};

public static final IClientConfigKey<Integer> ConnIdleEvictTimeMilliSeconds = new CommonClientConfigKey<Integer>("ConnIdleEvictTimeMilliSeconds"){};
public static final IClientConfigKey<Integer> ConnIdleEvictTimeMilliSeconds = new CommonClientConfigKey<Integer>("ConnIdleEvictTimeMilliSeconds", 30*1000){};

public static final IClientConfigKey<Integer> ConnectionCleanerRepeatInterval = new CommonClientConfigKey<Integer>("ConnectionCleanerRepeatInterval"){};
public static final IClientConfigKey<Integer> ConnectionCleanerRepeatInterval = new CommonClientConfigKey<Integer>("ConnectionCleanerRepeatInterval", 30*1000){};

public static final IClientConfigKey<Boolean> EnableGZIPContentEncodingFilter = new CommonClientConfigKey<Boolean>("EnableGZIPContentEncodingFilter", false){};

Expand Down Expand Up @@ -282,6 +282,6 @@ public String toString() {
}

@Override
public T getDefaultValue() { return defaultValue; }
public T defaultValue() { return defaultValue; }

}
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,20 @@ public interface IClientConfig {
* <br><br>
*/
default <T> T getOrDefault(IClientConfigKey<T> key) {
return get(key, key.getDefaultValue());
return get(key, key.defaultValue());
}

/**
* @return Return a global dynamic property not scoped to the specific client. The property will be looked up as is using the
* key without any client name or namespace prefix
*/
<T> Property<T> getGlobalProperty(IClientConfigKey<T> key);

/**
* @return Return a dynamic property scoped to the client name or namespace.
*/
<T> Property<T> getDynamicProperty(IClientConfigKey<T> key);

/**
* Returns a typed property. If the property of IClientConfigKey is not set,
* it returns the default value passed in as the parameter.
Expand All @@ -119,7 +130,7 @@ default <T> T getOrDefault(IClientConfigKey<T> key) {
* Set the typed property with the given value.
*/
<T> IClientConfig set(IClientConfigKey<T> key, T value);

class Builder {

private IClientConfig config;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,45 @@ private Keys(String configKey) {
*/
Class<T> type();

default T getDefaultValue() { return null; }
default T defaultValue() { return null; }

default IClientConfigKey<T> format(Object ... args) {
return create(String.format(key(), args), type(), defaultValue());
}

default IClientConfigKey<T> create(String key, Class<T> type, T defaultValue) {
return new IClientConfigKey<T>() {

@Override
public int hashCode() {
return key().hashCode();
}

@Override
public boolean equals(Object obj) {
if (obj instanceof IClientConfigKey) {
return key().equals(((IClientConfigKey)obj).key());
}
return false;
}

@Override
public String toString() {
return key();
}

@Override
public String key() {
return key;
}

@Override
public Class<T> type() {
return type;
}

@Override
public T defaultValue() { return defaultValue; }
};
}
}
34 changes: 34 additions & 0 deletions ribbon-core/src/main/java/com/netflix/client/config/Property.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.netflix.client.config;

import java.util.function.Consumer;

/**
* Ribbon specific encapsulation of a dynamic configuration property
* @param <T>
*/
public interface Property<T> {
/**
* Register a consumer to be called when the configuration changes
* @param consumer
*/
void onChange(Consumer<T> consumer);

/**
* @return Get the current value or default value
*/
T get();

static <T> Property<T> of(T value) {
return new Property<T>() {
@Override
public void onChange(Consumer<T> consumer) {

}

@Override
public T get() {
return value;
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.netflix.client.config;

public class UnboxedIntProperty {
private volatile int value;

public UnboxedIntProperty(Property<Integer> delegate) {
this.value = delegate.get();

delegate.onChange(newValue -> this.value = newValue);
}

public UnboxedIntProperty(int constantValue) {
this.value = constantValue;
}

public int get() {
return value;
}
}
4 changes: 2 additions & 2 deletions ribbon-eureka/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ dependencies {
compile project(':ribbon-loadbalancer')
compile "com.netflix.eureka:eureka-client:${eureka_version}"
compile 'com.google.code.findbugs:annotations:2.0.0'
compile 'org.slf4j:slf4j-api:1.6.4'
compile "com.netflix.archaius:archaius-core:${archaius_version}"
compile "org.slf4j:slf4j-api:${slf4j_version}"

testCompile project(":ribbon-archaius")
testCompile "org.slf4j:slf4j-log4j12:${slf4j_version}"
testCompile "junit:junit:${junit_version}"
testCompile "org.powermock:powermock-easymock-release-full:${powermock_version}"
testCompile "org.powermock:powermock-mockito-release-full:${powermock_version}"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class DiscoveryEnabledNIWSServerList extends AbstractServerList<Discovery
String datacenter;
String targetRegion;

int overridePort = CommonClientConfigKey.Port.getDefaultValue();
int overridePort = CommonClientConfigKey.Port.defaultValue();
boolean shouldUseOverridePort = false;
boolean shouldUseIpAddr = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.ArrayList;
import java.util.List;

import com.netflix.config.ConfigurationBasedDeploymentContext;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

Expand All @@ -40,7 +42,7 @@
public class DefaultNIWSServerListFilterTest {
@BeforeClass
public static void init() {
ConfigurationManager.getDeploymentContext().setValue(ContextKey.zone, "us-eAst-1C");
ConfigurationManager.getConfigInstance().setProperty(ContextKey.zone.getKey(), "us-eAst-1C");
}

private DiscoveryEnabledServer createServer(String host, String zone) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.netflix.loadbalancer.ZoneAwareLoadBalancer;
import org.apache.commons.configuration.Configuration;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.api.easymock.PowerMock;
Expand Down Expand Up @@ -79,10 +80,11 @@ public void setupMock(){
}

@Test
@Ignore
public void testBuildWithDiscoveryEnabledNIWSServerList() {
IRule rule = new AvailabilityFilteringRule();
ServerList<DiscoveryEnabledServer> list = new DiscoveryEnabledNIWSServerList("dummy:7001");
ServerListFilter<DiscoveryEnabledServer> filter = new ZoneAffinityServerListFilter<DiscoveryEnabledServer>();
ServerListFilter<DiscoveryEnabledServer> filter = new ZoneAffinityServerListFilter<>();
ZoneAwareLoadBalancer<DiscoveryEnabledServer> lb = LoadBalancerBuilder.<DiscoveryEnabledServer>newBuilder()
.withDynamicServerList(list)
.withRule(rule)
Expand All @@ -98,10 +100,11 @@ public void testBuildWithDiscoveryEnabledNIWSServerList() {
}

@Test
@Ignore
public void testBuildWithDiscoveryEnabledNIWSServerListAndUpdater() {
IRule rule = new AvailabilityFilteringRule();
ServerList<DiscoveryEnabledServer> list = new DiscoveryEnabledNIWSServerList("dummy:7001");
ServerListFilter<DiscoveryEnabledServer> filter = new ZoneAffinityServerListFilter<DiscoveryEnabledServer>();
ServerListFilter<DiscoveryEnabledServer> filter = new ZoneAffinityServerListFilter<>();
ServerListUpdater updater = new PollingServerListUpdater();
ZoneAwareLoadBalancer<DiscoveryEnabledServer> lb = LoadBalancerBuilder.<DiscoveryEnabledServer>newBuilder()
.withDynamicServerList(list)
Expand Down
4 changes: 4 additions & 0 deletions ribbon-eureka/src/test/resources/log4j.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
log4j.rootCategory=INFO,stdout
log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=%d %-5p %C:%L [%t] [%M] %m%n
2 changes: 1 addition & 1 deletion ribbon-evcache/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ dependencies {
compile project(':ribbon')
compile 'com.netflix.evcache:evcache-client:1.0.5'
compile "io.reactivex:rxjava:${rx_java_version}"
compile 'org.slf4j:slf4j-api:1.6.4'
compile "org.slf4j:slf4j-api:${slf4j_version}"
testCompile project(':ribbon-test')
testCompile project(path: ':ribbon', configuration: 'test')
testCompile "junit:junit:${junit_version}"
Expand Down
7 changes: 3 additions & 4 deletions ribbon-httpclient/build.gradle
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
dependencies {
compile project(':ribbon-core')
compile project(':ribbon-archaius')
compile project(':ribbon-loadbalancer')
compile 'commons-collections:commons-collections:3.2.2'
compile 'org.apache.httpcomponents:httpclient:4.2.1'
compile 'com.google.code.findbugs:annotations:2.0.0'
compile "com.sun.jersey:jersey-client:${jersey_version}"
compile "com.sun.jersey.contribs:jersey-apache-client4:${jersey_version}"
compile 'org.slf4j:slf4j-api:1.6.4'
compile "org.slf4j:slf4j-api:${slf4j_version}"
compile "com.netflix.servo:servo-core:${servo_version}"
compile "com.google.guava:guava:${guava_version}"
compile "com.netflix.archaius:archaius-core:${archaius_version}"
compile 'com.netflix.netflix-commons:netflix-commons-util:0.1.1'
testCompile 'junit:junit:4.11'
testCompile 'org.slf4j:slf4j-log4j12:1.7.2'
testCompile "org.slf4j:slf4j-log4j12:${slf4j_version}"
testCompile 'commons-io:commons-io:2.0.1'
testCompile "com.sun.jersey:jersey-server:${jersey_version}"
testCompile 'com.google.mockwebserver:mockwebserver:20130505'
testCompile project(':ribbon-archaius')
testCompile project(":ribbon-loadbalancer").sourceSets.test.output
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,14 @@
*/
package com.netflix.http4;

import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

import com.netflix.client.config.Property;
import org.apache.http.conn.ClientConnectionManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.netflix.config.DynamicIntProperty;
import com.netflix.config.DynamicPropertyFactory;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

/**
* Class that is responsible to cleanup connections based on a policy
Expand All @@ -41,14 +39,12 @@ public class ConnectionPoolCleaner {
String name = "default";
ClientConnectionManager connMgr;
ScheduledExecutorService scheduler;

private DynamicIntProperty connIdleEvictTimeMilliSeconds
= DynamicPropertyFactory.getInstance().getIntProperty("default.nfhttpclient.connIdleEvictTimeMilliSeconds",
NFHttpClientConstants.DEFAULT_CONNECTIONIDLE_TIME_IN_MSECS);

private Property<Integer> connIdleEvictTimeMilliSeconds = Property.of(30*1000);

volatile boolean enableConnectionPoolCleanerTask = false;
long connectionCleanerTimerDelay = 10;
long connectionCleanerRepeatInterval = NFHttpClientConstants.DEFAULT_CONNECTION_IDLE_TIMERTASK_REPEAT_IN_MSECS;
long connectionCleanerRepeatInterval = 30*1000;
private volatile ScheduledFuture<?> scheduledFuture;

public ConnectionPoolCleaner(String name, ClientConnectionManager connMgr, ScheduledExecutorService scheduler){
Expand All @@ -57,12 +53,11 @@ public ConnectionPoolCleaner(String name, ClientConnectionManager connMgr, Sched
this.scheduler = scheduler;
}

public DynamicIntProperty getConnIdleEvictTimeMilliSeconds() {
public Property<Integer> getConnIdleEvictTimeMilliSeconds() {
return connIdleEvictTimeMilliSeconds;
}

public void setConnIdleEvictTimeMilliSeconds(
DynamicIntProperty connIdleEvictTimeMilliSeconds) {
public void setConnIdleEvictTimeMilliSeconds(Property<Integer> connIdleEvictTimeMilliSeconds) {
this.connIdleEvictTimeMilliSeconds = connIdleEvictTimeMilliSeconds;
}

Expand Down
Loading

0 comments on commit 8d9d700

Please sign in to comment.