Skip to content

S
spring-cloud-netflix
Commit 16155ed8 by Patrick Way Committed by Ryan Baxter
Options

Fix npe when no servers available (#2727)

* Retryable Clients should validate the service server and throw an exception if necessary * fixing spelling error * RetryableRibbonLoadBalancingClient should throw a ClientException which can be handled by the RibbonRoutingFilter * moving service instance validation and service instance chooser to the AbstractLoadBalancingClient * Updating RetryCallback within RetryableRibbonLoadBalancingHttpClient to handel Exception
parent c84d20cd
Showing with 236 additions and 40 deletions
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java
......@@ -138,7 +138,7 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
serverStats.addToFailureCount();
LOGGER.debug(lbServer.getHostPort() + " RetryCount: " + context.getRetryCount()
+ " Successive Failures: " + serverStats.getSuccessiveConnectionFailureCount()
+ " CirtuitBreakerTripped:" + serverStats.isCircuitBreakerTripped());
+ " CircuitBreakerTripped:" + serverStats.isCircuitBreakerTripped());
}
}
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java
......@@ -44,6 +44,8 @@ import org.springframework.retry.backoff.NoBackOffPolicy;
import org.springframework.retry.policy.NeverRetryPolicy;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.ClientException;
import com.netflix.client.RequestSpecificRetryHandler;
import com.netflix.client.RetryHandler;
import com.netflix.client.config.CommonClientConfigKey;
......@@ -55,8 +57,7 @@ import com.netflix.loadbalancer.Server;
* @author Ryan Baxter
* @author Gang Li
*/
public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient
implements ServiceInstanceChooser {
public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient {
private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory();
private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory =
new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory();
......@@ -116,22 +117,21 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
final RequestConfig requestConfig = builder.build();
final LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this);
RetryCallback<RibbonApacheHttpResponse, IOException> retryCallback = new RetryCallback<RibbonApacheHttpResponse, IOException>() {
RetryCallback<RibbonApacheHttpResponse, Exception> retryCallback = new RetryCallback<RibbonApacheHttpResponse, Exception>() {
@Override
public RibbonApacheHttpResponse doWithRetry(RetryContext context) throws IOException {
public RibbonApacheHttpResponse doWithRetry(RetryContext context) throws Exception {
//on retries the policy will choose the server and set it in the context
//extract the server and update the request being made
RibbonApacheHttpRequest newRequest = request;
if(context instanceof LoadBalancedRetryContext) {
ServiceInstance service = ((LoadBalancedRetryContext)context).getServiceInstance();
if(service != null) {
//Reconstruct the request URI using the host and port set in the retry context
newRequest = newRequest.withNewUri(UriComponentsBuilder.newInstance().host(service.getHost())
.scheme(service.getUri().getScheme()).userInfo(newRequest.getURI().getUserInfo())
.port(service.getPort()).path(newRequest.getURI().getPath())
.query(newRequest.getURI().getQuery()).fragment(newRequest.getURI().getFragment())
.build().encode().toUri());
}
validateServiceInstance(service);
//Reconstruct the request URI using the host and port set in the retry context
newRequest = newRequest.withNewUri(UriComponentsBuilder.newInstance().host(service.getHost())
.scheme(service.getUri().getScheme()).userInfo(newRequest.getURI().getUserInfo())
.port(service.getPort()).path(newRequest.getURI().getPath())
.query(newRequest.getURI().getQuery()).fragment(newRequest.getURI().getFragment())
.build().encode().toUri());
}
newRequest = getSecureRequest(newRequest, configOverride);
HttpUriRequest httpUriRequest = newRequest.toRequest(requestConfig);
......@@ -163,7 +163,7 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
}
private RibbonApacheHttpResponse executeWithRetry(RibbonApacheHttpRequest request, LoadBalancedRetryPolicy retryPolicy,
RetryCallback<RibbonApacheHttpResponse, IOException> callback,
RetryCallback<RibbonApacheHttpResponse, Exception> callback,
RecoveryCallback<RibbonApacheHttpResponse> recoveryCallback) throws Exception {
RetryTemplate retryTemplate = new RetryTemplate();
boolean retryable = isRequestRetryable(request);
......@@ -179,12 +179,6 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
}
@Override
public ServiceInstance choose(String serviceId) {
Server server = this.getLoadBalancer().chooseServer(serviceId);
return new RibbonLoadBalancerClient.RibbonServer(serviceId, server);
}
@Override
public RequestSpecificRetryHandler getRequestSpecificRetryHandler(RibbonApacheHttpRequest request, IClientConfig requestConfig) {
return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null);
}
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java
......@@ -44,6 +44,8 @@ import org.springframework.retry.backoff.NoBackOffPolicy;
import org.springframework.retry.policy.NeverRetryPolicy;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.ClientException;
import com.netflix.client.RequestSpecificRetryHandler;
import com.netflix.client.RetryHandler;
import com.netflix.client.config.IClientConfig;
......@@ -54,7 +56,7 @@ import com.netflix.loadbalancer.Server;
* @author Ryan Baxter
* @author Gang Li
*/
public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient implements ServiceInstanceChooser {
public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient {
private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory;
private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory =
......@@ -128,13 +130,12 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
OkHttpRibbonRequest newRequest = ribbonRequest;
if(context instanceof LoadBalancedRetryContext) {
ServiceInstance service = ((LoadBalancedRetryContext)context).getServiceInstance();
if(service != null) {
//Reconstruct the request URI using the host and port set in the retry context
newRequest = newRequest.withNewUri(new URI(service.getUri().getScheme(),
newRequest.getURI().getUserInfo(), service.getHost(), service.getPort(),
newRequest.getURI().getPath(), newRequest.getURI().getQuery(),
newRequest.getURI().getFragment()));
}
validateServiceInstance(service);
//Reconstruct the request URI using the host and port set in the retry context
newRequest = newRequest.withNewUri(new URI(service.getUri().getScheme(),
newRequest.getURI().getUserInfo(), service.getHost(), service.getPort(),
newRequest.getURI().getPath(), newRequest.getURI().getQuery(),
newRequest.getURI().getFragment()));
}
if (isSecure(configOverride)) {
final URI secureUri = UriComponentsBuilder.fromUri(newRequest.getUri())
......@@ -163,12 +164,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
});
}
@Override
public ServiceInstance choose(String serviceId) {
Server server = this.getLoadBalancer().chooseServer(serviceId);
return new RibbonLoadBalancerClient.RibbonServer(serviceId,
server);
}
@Override
public RequestSpecificRetryHandler getRequestSpecificRetryHandler(OkHttpRibbonRequest request, IClientConfig requestConfig) {
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java
......@@ -18,11 +18,16 @@
package org.springframework.cloud.netflix.ribbon.support;
import com.netflix.loadbalancer.reactive.LoadBalancerCommand;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import com.netflix.client.AbstractLoadBalancerAwareClient;
import com.netflix.client.ClientException;
import com.netflix.client.IResponse;
import com.netflix.client.RequestSpecificRetryHandler;
import com.netflix.client.RetryHandler;
......@@ -30,12 +35,13 @@ import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
/**
* @author Spencer Gibb
*/
public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest, T extends IResponse, D> extends
AbstractLoadBalancerAwareClient<S, T> {
AbstractLoadBalancerAwareClient<S, T> implements ServiceInstanceChooser {
protected int connectTimeout;
......@@ -54,7 +60,7 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
public boolean isClientRetryable(ContextAwareRequest request) {
return false;
}
@Deprecated
public AbstractLoadBalancingClient() {
super(null);
......@@ -152,4 +158,21 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
builder.withServerLocator(request.getLoadBalancerKey());
}
}
@Override
public ServiceInstance choose(String serviceId) {
Server server = this.getLoadBalancer().chooseServer(serviceId);
if (server != null) {
return new RibbonLoadBalancerClient.RibbonServer(serviceId, server);
}
return null;
}
public void validateServiceInstance(ServiceInstance serviceInstance) throws ClientException {
if (serviceInstance == null) {
throw new ClientException("Load balancer does not have available server for client: " + clientName);
} else if (serviceInstance.getHost() == null) {
throw new ClientException("Invalid Server for: " + serviceInstance.getServiceId() + " null Host");
}
}
}
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java
......@@ -64,6 +64,7 @@ import org.springframework.retry.backoff.BackOffPolicy;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.util.LinkedMultiValueMap;
import com.netflix.client.ClientException;
import com.netflix.client.DefaultLoadBalancerRetryHandler;
import com.netflix.client.RetryHandler;
import com.netflix.client.config.CommonClientConfigKey;
......@@ -74,6 +75,7 @@ import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
......@@ -732,6 +734,81 @@ public class RibbonLoadBalancingHttpClientTests {
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
}
private RetryableRibbonLoadBalancingHttpClient setupClientForServerValidation(String serviceName, String host, int port,
CloseableHttpClient delegate, ILoadBalancer lb) throws Exception {
ServerIntrospector introspector = mock(ServerIntrospector.class);
RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(1, 1, true);
DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, true);
clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, 0);
clientConfig.set(CommonClientConfigKey.MaxAutoRetries, 1);
clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, "");
clientConfig.set(CommonClientConfigKey.IsSecure, false);
clientConfig.setClientName(serviceName);
RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler);
SpringClientFactory clientFactory = mock(SpringClientFactory.class);
doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName));
doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName));
LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory);
RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(delegate,
clientConfig, introspector, factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory);
client.setLoadBalancer(lb);
ReflectionTestUtils.setField(client, "delegate", delegate);
return client;
}
@Test
public void noServersFoundTest() throws Exception {
String serviceName = "noservers";
String host = serviceName;
int port = 80;
HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port);
CloseableHttpClient delegate = mock(CloseableHttpClient.class);
ILoadBalancer lb = mock(ILoadBalancer.class);
RetryableRibbonLoadBalancingHttpClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class);
doReturn(null).when(lb).chooseServer(eq(serviceName));
doReturn(method).when(request).getMethod();
doReturn(uri).when(request).getURI();
doReturn(request).when(request).withNewUri(any(URI.class));
HttpUriRequest uriRequest = mock(HttpUriRequest.class);
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
try {
client.execute(request, null);
fail("Expected IOException for no servers available");
} catch (ClientException ex) {
assertThat(ex.getMessage(), containsString("Load balancer does not have available server for client"));
}
}
@Test
public void invalidServerTest() throws Exception {
String serviceName = "noservers";
String host = serviceName;
int port = 80;
HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port);
CloseableHttpClient delegate = mock(CloseableHttpClient.class);
ILoadBalancer lb = mock(ILoadBalancer.class);
RetryableRibbonLoadBalancingHttpClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class);
doReturn(new Server(null,8000)).when(lb).chooseServer(eq(serviceName));
doReturn(method).when(request).getMethod();
doReturn(uri).when(request).getURI();
doReturn(request).when(request).withNewUri(any(URI.class));
HttpUriRequest uriRequest = mock(HttpUriRequest.class);
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
try {
client.execute(request, null);
fail("Expected IOException for no servers available");
} catch (ClientException ex) {
assertThat(ex.getMessage(), containsString("Invalid Server for: "));
}
}
@Configuration
protected static class UseDefaults {
......
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java
......@@ -15,25 +15,52 @@
*/
package org.springframework.cloud.netflix.ribbon.okhttp;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import java.net.URI;
import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.BeansException;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory;
import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration;
import org.springframework.cloud.commons.httpclient.HttpClientConfiguration;
import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration;
import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicy;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerContext;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.http.HttpMethod;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.util.ReflectionTestUtils;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import com.netflix.client.ClientException;
import com.netflix.client.DefaultLoadBalancerRetryHandler;
import com.netflix.client.RetryHandler;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
import okhttp3.OkHttpClient;
import okhttp3.Request;
/**
* @author Ryan Baxter
......@@ -46,7 +73,10 @@ import static org.hamcrest.Matchers.instanceOf;
public class SpringRetryEnabledOkHttpClientTests implements ApplicationContextAware {
private ApplicationContext context;
private ILoadBalancer loadBalancer;
private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory();
private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory();
@Test
public void testLoadBalancedRetryFactoryBean() throws Exception {
Map<String, LoadBalancedRetryPolicyFactory> factories = context
......@@ -65,4 +95,80 @@ public class SpringRetryEnabledOkHttpClientTests implements ApplicationContextAw
public void setApplicationContext(ApplicationContext context) throws BeansException {
this.context = context;
}
private RetryableOkHttpLoadBalancingClient setupClientForServerValidation(String serviceName, String host, int port,
OkHttpClient delegate, ILoadBalancer lb) throws Exception {
ServerIntrospector introspector = mock(ServerIntrospector.class);
RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(1, 1, true);
DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, true);
clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, 0);
clientConfig.set(CommonClientConfigKey.MaxAutoRetries, 1);
clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, "");
clientConfig.set(CommonClientConfigKey.IsSecure, false);
clientConfig.setClientName(serviceName);
RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler);
SpringClientFactory clientFactory = mock(SpringClientFactory.class);
doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName));
doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName));
LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory);
RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient(delegate, clientConfig, introspector,
factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory);
client.setLoadBalancer(lb);
ReflectionTestUtils.setField(client, "delegate", delegate);
return client;
}
@Test
public void noServersFoundTest() throws Exception {
String serviceName = "noservers";
String host = serviceName;
int port = 80;
HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port);
OkHttpClient delegate = mock(OkHttpClient.class);
ILoadBalancer lb = mock(ILoadBalancer.class);
RetryableOkHttpLoadBalancingClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
OkHttpRibbonRequest request = mock(OkHttpRibbonRequest.class);
doReturn(null).when(lb).chooseServer(eq(serviceName));
doReturn(method).when(request).getMethod();
doReturn(uri).when(request).getURI();
doReturn(request).when(request).withNewUri(any(URI.class));
Request okRequest = new Request.Builder().url("ws:testerror.sc").build();
doReturn(okRequest).when(request).toRequest();
try {
client.execute(request, null);
fail("Expected ClientException for no servers available");
} catch (ClientException ex) {
assertThat(ex.getMessage(), containsString("Load balancer does not have available server for client"));
}
}
@Test
public void invalidServerTest() throws Exception {
String serviceName = "noservers";
String host = serviceName;
int port = 80;
HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port);
OkHttpClient delegate = mock(OkHttpClient.class);
ILoadBalancer lb = mock(ILoadBalancer.class);
RetryableOkHttpLoadBalancingClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
OkHttpRibbonRequest request = mock(OkHttpRibbonRequest.class);
doReturn(new Server(null,8000)).when(lb).chooseServer(eq(serviceName));
doReturn(method).when(request).getMethod();
doReturn(uri).when(request).getURI();
doReturn(request).when(request).withNewUri(any(URI.class));
Request okRequest = new Request.Builder().url("ws:testerror.sc").build();
doReturn(okRequest).when(request).toRequest();
try {
client.execute(request, null);
fail("Expected ClientException for no Invalid Host");
} catch (ClientException ex) {
assertThat(ex.getMessage(), containsString("Invalid Server for: "));
}
}
}
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