Skip to content

S
spring-cloud-netflix
Unverified Commit 05351b78 by Tyler Van Gorder Committed by Spencer Gibb
Options

Ribbon retry works & fix Content-Length error.

This change requires setting `ribbon.http.client.enabled=true`. This change addresses issue #648 Getting Retry Working In RestTemplate. There is also a fix in place that addresses the issue #357 RestTemplate Error. fixes gh-648
parent a3ab3259
Showing with 381 additions and 90 deletions
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java
......@@ -79,9 +79,6 @@ public class RibbonAutoConfiguration {
@Autowired
private SpringClientFactory springClientFactory;
@Autowired
private LoadBalancerClient loadBalancerClient;
@Bean
public RestTemplateCustomizer restTemplateCustomizer(
final RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory) {
......@@ -95,8 +92,7 @@ public class RibbonAutoConfiguration {
@Bean
public RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory() {
return new RibbonClientHttpRequestFactory(this.springClientFactory,
this.loadBalancerClient);
return new RibbonClientHttpRequestFactory(this.springClientFactory);
}
}
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java
......@@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.ribbon;
import java.net.URI;
import javax.annotation.PostConstruct;
import org.apache.http.client.params.ClientPNames;
import org.apache.http.client.params.CookiePolicy;
import org.springframework.beans.factory.annotation.Value;
......@@ -48,6 +50,9 @@ import com.netflix.servo.monitor.Monitors;
import com.sun.jersey.api.client.Client;
import com.sun.jersey.client.apache4.ApacheHttpClient4;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty;
/**
* @author Dave Syer
*/
......@@ -155,6 +160,11 @@ public class RibbonClientConfiguration {
return new DefaultServerIntrospector();
}
@PostConstruct
public void preprocess() {
setRibbonProperty(name, DeploymentContextBasedVipAddresses.key(), name);
}
static class OverrideRestClient extends RestClient {
private ServerIntrospector serverIntrospector;
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientHttpRequestFactory.java
......@@ -19,16 +19,12 @@ package org.springframework.cloud.netflix.ribbon;
import java.io.IOException;
import java.net.URI;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient.RibbonServer;
import org.springframework.http.HttpMethod;
import org.springframework.http.client.ClientHttpRequest;
import org.springframework.http.client.ClientHttpRequestFactory;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest;
import com.netflix.loadbalancer.Server;
import com.netflix.niws.client.http.RestClient;
/**
......@@ -38,12 +34,8 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory
private final SpringClientFactory clientFactory;
private LoadBalancerClient loadBalancer;
public RibbonClientHttpRequestFactory(SpringClientFactory clientFactory,
LoadBalancerClient loadBalancer) {
public RibbonClientHttpRequestFactory(SpringClientFactory clientFactory) {
this.clientFactory = clientFactory;
this.loadBalancer = loadBalancer;
}
@Override
......@@ -55,28 +47,11 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory
throw new IOException(
"Invalid hostname in the URI [" + originalUri.toASCIIString() + "]");
}
ServiceInstance instance = this.loadBalancer.choose(serviceId);
if (instance == null) {
throw new IllegalStateException("No instances available for " + serviceId);
}
URI uri = this.loadBalancer.reconstructURI(instance, originalUri);
IClientConfig clientConfig = this.clientFactory
.getClientConfig(instance.getServiceId());
RestClient client = this.clientFactory.getClient(instance.getServiceId(),
RestClient.class);
IClientConfig clientConfig = this.clientFactory.getClientConfig(serviceId);
RestClient client = this.clientFactory.getClient(serviceId, RestClient.class);
HttpRequest.Verb verb = HttpRequest.Verb.valueOf(httpMethod.name());
RibbonLoadBalancerContext context = this.clientFactory
.getLoadBalancerContext(serviceId);
Server server = null;
if (instance instanceof RibbonServer) {
server = ((RibbonServer) instance).getServer();
}
RibbonStatsRecorder statsRecorder = new RibbonStatsRecorder(context, server);
return new RibbonHttpRequest(uri, verb, client, clientConfig, statsRecorder);
return new RibbonHttpRequest(originalUri, verb, client, clientConfig);
}
}
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java
......@@ -16,21 +16,22 @@
package org.springframework.cloud.netflix.ribbon;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest;
import com.netflix.client.http.HttpResponse;
import com.netflix.niws.client.http.RestClient;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.client.AbstractClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
import java.util.List;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.client.AbstractClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest;
import com.netflix.client.http.HttpResponse;
import com.netflix.niws.client.http.RestClient;
/**
* @author Spencer Gibb
*/
......@@ -42,16 +43,14 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest {
private HttpRequest.Verb verb;
private RestClient client;
private IClientConfig config;
private RibbonStatsRecorder statsRecorder;
private ByteArrayOutputStream outputStream = null;
public RibbonHttpRequest(URI uri, HttpRequest.Verb verb, RestClient client,
IClientConfig config, RibbonStatsRecorder statsRecorder) {
IClientConfig config) {
this.uri = uri;
this.verb = verb;
this.client = client;
this.config = config;
this.statsRecorder = statsRecorder;
this.builder = HttpRequest.newBuilder().uri(uri).verb(verb);
}
......@@ -83,24 +82,27 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest {
builder.entity(outputStream.toByteArray());
}
HttpRequest request = builder.build();
HttpResponse response = client.execute(request, config);
statsRecorder.recordStats(response);
HttpResponse response = client.executeWithLoadBalancer(request, config);
return new RibbonHttpResponse(response);
} catch (Exception e) {
statsRecorder.recordStats(e);
throw new IOException(e);
}
}
private void addHeaders(HttpHeaders headers) {
for (String name : headers.keySet()) {
// apache http RequestContent pukes if there is a body and
// the dynamic headers are already present
if (!isDynamic(name)) {
List<String> values = headers.get(name);
for (String value : values) {
builder.header(name, value);
}
// apache http RequestContent pukes if there is a body and
// the dynamic headers are already present
if (isDynamic(name) && outputStream != null) {
continue;
}
//Don't add content-length if the output stream is null. The RibbonClient does this for us.
if (name.equals("Content-Length") && outputStream == null) {
continue;
}
List<String> values = headers.get(name);
for (String value : values) {
builder.header(name, value);
}
}
}
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonProperyUtils.java 0 → 100644
package org.springframework.cloud.netflix.ribbon;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty;
/**
* @author Spencer Gibb
*/
public class RibbonProperyUtils {
public static final String VALUE_NOT_SET = "__not__set__";
public static final String DEFAULT_NAMESPACE = "ribbon";
public static void setRibbonProperty(String serviceId, String suffix, String value) {
// how to set the namespace properly?
String key = getRibbonKey(serviceId, suffix);
DynamicStringProperty property = getProperty(key);
if (property.get().equals(VALUE_NOT_SET)) {
ConfigurationManager.getConfigInstance().setProperty(key, value);
}
}
public static String getRibbonKey(String serviceId, String suffix) {
return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix;
}
public static DynamicStringProperty getProperty(String key) {
return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET);
}
}
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTest.java 0 → 100644
package org.springframework.cloud.netflix.resttemplate;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.IntegrationTest;
import org.springframework.boot.test.SpringApplicationConfiguration;
import org.springframework.cloud.client.loadbalancer.LoadBalanced;
import org.springframework.cloud.netflix.ribbon.RibbonClient;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.util.Assert;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.RestTemplate;
import com.netflix.client.RetryHandler;
import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.AvailabilityFilteringRule;
import com.netflix.loadbalancer.BaseLoadBalancer;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.IPing;
import com.netflix.loadbalancer.IRule;
import com.netflix.loadbalancer.LoadBalancerBuilder;
import com.netflix.loadbalancer.LoadBalancerStats;
import com.netflix.loadbalancer.Server;
import com.netflix.loadbalancer.ServerList;
import com.netflix.loadbalancer.ServerStats;
import com.netflix.niws.client.http.HttpClientLoadBalancerErrorHandler;
@RunWith(SpringJUnit4ClassRunner.class)
@SpringApplicationConfiguration(classes = RestTemplateRetryTest.Application.class)
@WebAppConfiguration
@IntegrationTest({
"server.port=0",
"spring.application.name=resttemplatetest",
"logging.level.org.springframework.cloud.netflix.resttemplate=DEBUG",
"badClients.ribbon.MaxAutoRetries=0", "badClients.ribbon.ReadTimeout=200",
"badClients.ribbon.MaxAutoRetriesNextServer=10",
"badClients.ribbon.OkToRetryOnAllOperations=true", "ribbon.http.client.enabled" })
@DirtiesContext
public class RestTemplateRetryTest {
final private static Log logger = LogFactory.getLog(RestTemplateRetryTest.class);
@Value("${local.server.port}")
private int port = 0;
@Autowired
private RestTemplate testClient;
public RestTemplateRetryTest() {
}
@Configuration
@EnableAutoConfiguration
@RestController
@RibbonClient(name = "badClients", configuration = LocalBadClientConfiguration.class)
public static class Application {
private AtomicInteger hits = new AtomicInteger(1);
private AtomicInteger retryHits = new AtomicInteger(1);
@RequestMapping(method = RequestMethod.GET, value = "/ping")
public int ping() {
return 0;
}
@RequestMapping(method = RequestMethod.GET, value = "/good")
public int good() {
int lValue = hits.getAndIncrement();
return lValue;
}
@RequestMapping(method = RequestMethod.GET, value = "/timeout")
public int timeout() throws Exception {
int lValue = retryHits.getAndIncrement();
// Force the good server to have 2 consecutive errors a couple of times.
if (lValue == 2 || lValue == 3 || lValue == 5 || lValue == 6) {
Thread.sleep(500);
}
return lValue;
}
@RequestMapping(method = RequestMethod.GET, value = "/null")
public int isNull() throws Exception {
throw new NullPointerException("Null");
}
@LoadBalanced
@Bean
RestTemplate restTemplate() {
return new RestTemplate();
}
}
@Before
public void setup() throws Exception {
// Force Ribbon configuration by making one call.
testClient.getForObject("http://badClients/ping", Integer.class);
}
@Test
public void testNullPointer() throws Exception {
LoadBalancerStats stats = LocalBadClientConfiguration.balancer
.getLoadBalancerStats();
ServerStats badServer1Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer);
ServerStats badServer2Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer2);
ServerStats goodServerStats = stats
.getSingleServerStat(LocalBadClientConfiguration.goodServer);
badServer1Stats.clearSuccessiveConnectionFailureCount();
badServer2Stats.clearSuccessiveConnectionFailureCount();
long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 10;
// A null pointer should NOT trigger a circuit breaker.
for (int index = 0; index < 10; index++) {
try {
testClient.getForObject("http://badClients/null", Integer.class);
}
catch (Exception exception) {
}
}
logServerStats(LocalBadClientConfiguration.badServer);
logServerStats(LocalBadClientConfiguration.badServer2);
logServerStats(LocalBadClientConfiguration.goodServer);
Assert.isTrue(badServer1Stats.isCircuitBreakerTripped());
Assert.isTrue(badServer2Stats.isCircuitBreakerTripped());
Assert.isTrue(goodServerStats.getTotalRequestsCount() == targetConnectionCount);
// Wait for any timeout thread to finish.
}
private void logServerStats(Server server) {
LoadBalancerStats stats = LocalBadClientConfiguration.balancer
.getLoadBalancerStats();
ServerStats serverStats = stats.getSingleServerStat(server);
logger.debug("Server : " + server.toString() + " : Total Count == "
+ serverStats.getTotalRequestsCount() + ", Failure Count == "
+ serverStats.getFailureCount() + ", Successive Connection Failure == "
+ serverStats.getSuccessiveConnectionFailureCount()
+ ", Circuit Breaker ? == " + serverStats.isCircuitBreakerTripped());
}
@Test
public void testRestRetries() {
LoadBalancerStats stats = LocalBadClientConfiguration.balancer
.getLoadBalancerStats();
ServerStats badServer1Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer);
ServerStats badServer2Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer2);
ServerStats goodServerStats = stats
.getSingleServerStat(LocalBadClientConfiguration.goodServer);
badServer1Stats.clearSuccessiveConnectionFailureCount();
badServer2Stats.clearSuccessiveConnectionFailureCount();
long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 20;
int hits = 0;
for (int index = 0; index < 20; index++) {
hits = testClient.getForObject("http://badClients/good", Integer.class);
}
logServerStats(LocalBadClientConfiguration.badServer);
logServerStats(LocalBadClientConfiguration.badServer2);
logServerStats(LocalBadClientConfiguration.goodServer);
Assert.isTrue(badServer1Stats.isCircuitBreakerTripped());
Assert.isTrue(badServer2Stats.isCircuitBreakerTripped());
Assert.isTrue(goodServerStats.getTotalRequestsCount() == targetConnectionCount);
Assert.isTrue(hits == 20);
System.out.println("Retry Hits: " + hits);
}
@Test
public void testRestRetriesWithReadTimeout() throws Exception {
LoadBalancerStats stats = LocalBadClientConfiguration.balancer
.getLoadBalancerStats();
ServerStats badServer1Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer);
ServerStats badServer2Stats = stats
.getSingleServerStat(LocalBadClientConfiguration.badServer2);
ServerStats goodServerStats = stats
.getSingleServerStat(LocalBadClientConfiguration.goodServer);
badServer1Stats.clearSuccessiveConnectionFailureCount();
badServer2Stats.clearSuccessiveConnectionFailureCount();
Assert.isTrue(!badServer1Stats.isCircuitBreakerTripped());
Assert.isTrue(!badServer2Stats.isCircuitBreakerTripped());
int hits = 0;
for (int index = 0; index < 15; index++) {
hits = testClient.getForObject("http://badClients/timeout", Integer.class);
}
logServerStats(LocalBadClientConfiguration.badServer);
logServerStats(LocalBadClientConfiguration.badServer2);
logServerStats(LocalBadClientConfiguration.goodServer);
Assert.isTrue(badServer1Stats.isCircuitBreakerTripped());
Assert.isTrue(badServer2Stats.isCircuitBreakerTripped());
Assert.isTrue(!goodServerStats.isCircuitBreakerTripped());
// 15 + 4 timeouts. See the endpoint for timeout conditions.
Assert.isTrue(hits == 19);
// Wait for any timeout thread to finish.
Thread.sleep(600);
}
}
// Load balancer with fixed server list for "local" pointing to localhost
// and some bogus servers are thrown in to test retry
@Configuration
class LocalBadClientConfiguration {
static BaseLoadBalancer balancer;
static Server goodServer;
static Server badServer;
static Server badServer2;
public LocalBadClientConfiguration() {
}
@Value("${local.server.port}")
private int port = 0;
@Bean
public IRule loadBalancerRule() {
// This is a good place to try different load balancing rules and how those rules
// behave in failure
// states: BestAvailableRule, WeightedResponseTimeRule, etc
// This rule just uses a round robin and will skip servers that are in circuit
// breaker state.
return new AvailabilityFilteringRule();
}
@Bean
public ILoadBalancer ribbonLoadBalancer(IClientConfig config,
ServerList<Server> serverList, IRule rule, IPing ping) {
goodServer = new Server("localhost", this.port);
badServer = new Server("mybadhost", 10001);
badServer2 = new Server("localhost", -1);
balancer = LoadBalancerBuilder
.newBuilder()
.withClientConfig(config)
.withRule(rule)
.withPing(ping)
.buildFixedServerListLoadBalancer(
Arrays.asList(badServer, badServer2, goodServer));
return balancer;
}
@Bean
public RetryHandler retryHandler() {
return new OverrideRetryHandler();
}
static class OverrideRetryHandler extends HttpClientLoadBalancerErrorHandler {
public OverrideRetryHandler() {
circuitRelated.add(UnknownHostException.class);
retriable.add(UnknownHostException.class);
}
}
}
spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java
......@@ -16,13 +16,8 @@
package org.springframework.cloud.netflix.ribbon.eureka;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity;
import javax.annotation.PostConstruct;
import lombok.extern.apachecommons.CommonsLog;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
......@@ -35,14 +30,18 @@ import com.netflix.appinfo.EurekaInstanceConfig;
import com.netflix.client.config.IClientConfig;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DeploymentContext.ContextKey;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty;
import com.netflix.discovery.EurekaClientConfig;
import com.netflix.loadbalancer.IPing;
import com.netflix.loadbalancer.ServerList;
import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList;
import com.netflix.niws.loadbalancer.NIWSDiscoveryPing;
import lombok.extern.apachecommons.CommonsLog;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty;
/**
* Preprocessor that configures defaults for eureka-discovered ribbon clients. Such as:
* <code>@zone</code>, NIWSServerListClassName, DeploymentContextBasedVipAddresses,
......@@ -62,10 +61,6 @@ public class EurekaRibbonClientConfiguration {
@Value("${ribbon.client.name}")
private String serviceId = "client";
protected static final String VALUE_NOT_SET = "__not__set__";
protected static final String DEFAULT_NAMESPACE = "ribbon";
@Autowired(required = false)
private EurekaClientConfig clientConfig;
......@@ -132,25 +127,8 @@ public class EurekaRibbonClientConfiguration {
}
}
}
setProp(this.serviceId, DeploymentContextBasedVipAddresses.key(), this.serviceId);
setProp(this.serviceId, EnableZoneAffinity.key(), "true");
}
protected void setProp(String serviceId, String suffix, String value) {
// how to set the namespace properly?
String key = getKey(serviceId, suffix);
DynamicStringProperty property = getProperty(key);
if (property.get().equals(VALUE_NOT_SET)) {
ConfigurationManager.getConfigInstance().setProperty(key, value);
}
}
protected DynamicStringProperty getProperty(String key) {
return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET);
}
protected String getKey(String serviceId, String suffix) {
return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix;
setRibbonProperty(this.serviceId, DeploymentContextBasedVipAddresses.key(), this.serviceId);
setRibbonProperty(this.serviceId, EnableZoneAffinity.key(), "true");
}
}
spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java
......@@ -36,7 +36,8 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledServer;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.springframework.cloud.netflix.ribbon.eureka.EurekaRibbonClientConfiguration.VALUE_NOT_SET;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.*;
/**
* @author Dave Syer
......@@ -82,12 +83,12 @@ public class EurekaRibbonClientConfigurationTests {
String serviceId = "myService";
String suffix = "mySuffix";
String value = "myValue";
DynamicStringProperty property = preprocessor.getProperty(preprocessor.getKey(
DynamicStringProperty property = getProperty(getRibbonKey(
serviceId, suffix));
assertEquals("property doesn't have default value", VALUE_NOT_SET, property.get());
preprocessor.setProp(serviceId, suffix, value);
setRibbonProperty(serviceId, suffix, value);
assertEquals("property has wrong value", value, property.get());
preprocessor.setProp(serviceId, suffix, value);
setRibbonProperty(serviceId, suffix, value);
assertEquals("property has wrong value", value, property.get());
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment