Commit 35477d9c by Patrick Way Committed by Ryan Baxter

Allowing RibbonLoadbalancedRetryPolicy to update ServerStats for circuit…

Allowing RibbonLoadbalancedRetryPolicy to update ServerStats for circuit tripping exceptions (#2483) * Allowing RibbonLoadbalancedRetryPolicy to update ServerStats for circuit tripping exceptions
parent 576baf6f
......@@ -16,17 +16,25 @@
package org.springframework.cloud.netflix.ribbon;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.config.IClientConfigKey;
import java.util.ArrayList;
import java.util.List;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient.RibbonServer;
import org.springframework.http.HttpMethod;
import org.springframework.util.StringUtils;
import java.util.ArrayList;
import java.util.List;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.config.IClientConfigKey;
import com.netflix.loadbalancer.Server;
import com.netflix.loadbalancer.ServerStats;
/**
* {@link LoadBalancedRetryPolicy} for Ribbon clients.
......@@ -41,6 +49,8 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
private RibbonLoadBalancerContext lbContext;
private ServiceInstanceChooser loadBalanceChooser;
List<Integer> retryableStatusCodes = new ArrayList<>();
private static final Log LOGGER = LogFactory.getLog(RibbonLoadBalancedRetryPolicy.class);
public RibbonLoadBalancedRetryPolicy(String serviceId, RibbonLoadBalancerContext context, ServiceInstanceChooser loadBalanceChooser) {
this.serviceId = serviceId;
......@@ -91,6 +101,11 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
@Override
public void registerThrowable(LoadBalancedRetryContext context, Throwable throwable) {
//if this is a circuit tripping exception then notify the load balancer
if (lbContext.getRetryHandler().isCircuitTrippingException(throwable)) {
updateServerInstanceStats(context);
}
//Check if we need to ask the load balancer for a new server.
//Do this before we increment the counters because the first call to this method
//is not a retry it is just an initial failure.
......@@ -113,6 +128,19 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
}
}
private void updateServerInstanceStats(LoadBalancedRetryContext context) {
ServiceInstance serviceInstance = context.getServiceInstance();
if (serviceInstance instanceof RibbonServer) {
Server lbServer = ((RibbonServer)serviceInstance).getServer();
ServerStats serverStats = lbContext.getServerStats(lbServer);
serverStats.incrementSuccessiveConnectionFailureCount();
serverStats.addToFailureCount();
LOGGER.debug(lbServer.getHostPort() + " RetryCount: " + context.getRetryCount()
+ " Successive Failures: " + serverStats.getSuccessiveConnectionFailureCount()
+ " CirtuitBreakerTripped:" + serverStats.isCircuitBreakerTripped());
}
}
@Override
public boolean retryableStatusCode(int statusCode) {
......
......@@ -31,9 +31,10 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory;
import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException;
import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
import org.springframework.cloud.netflix.feign.ribbon.FeignRetryPolicy;
import org.springframework.cloud.client.loadbalancer.InterceptorRetryPolicy;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.support.ContextAwareRequest;
import org.springframework.http.HttpRequest;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext;
......@@ -147,11 +148,20 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
};
return this.executeWithRetry(request, retryPolicy, retryCallback);
}
@Override
public boolean isClientRetryable(ContextAwareRequest request) {
return request!= null && isRequestRetryable(request);
}
private boolean isRequestRetryable(ContextAwareRequest request) {
return request.getContext() == null ? true :
BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true);
}
private RibbonApacheHttpResponse executeWithRetry(RibbonApacheHttpRequest request, LoadBalancedRetryPolicy retryPolicy, RetryCallback<RibbonApacheHttpResponse, IOException> callback) throws Exception {
RetryTemplate retryTemplate = new RetryTemplate();
boolean retryable = request.getContext() == null ? true :
BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true);
boolean retryable = isRequestRetryable(request);
retryTemplate.setRetryPolicy(retryPolicy == null || !retryable ? new NeverRetryPolicy()
: new RetryPolicy(request, retryPolicy, this, this.getClientName()));
BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName());
......@@ -174,8 +184,9 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null);
}
static class RetryPolicy extends FeignRetryPolicy {
public RetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, ServiceInstanceChooser serviceInstanceChooser, String serviceName) {
static class RetryPolicy extends InterceptorRetryPolicy {
public RetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy,
ServiceInstanceChooser serviceInstanceChooser, String serviceName) {
super(request, policy, serviceInstanceChooser, serviceName);
}
}
......
......@@ -29,9 +29,10 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory;
import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException;
import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
import org.springframework.cloud.netflix.feign.ribbon.FeignRetryPolicy;
import org.springframework.cloud.client.loadbalancer.InterceptorRetryPolicy;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.support.ContextAwareRequest;
import org.springframework.http.HttpRequest;
import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext;
......@@ -87,6 +88,16 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory;
}
@Override
public boolean isClientRetryable(ContextAwareRequest request) {
return request!= null && isRequestRetryable(request);
}
private boolean isRequestRetryable(ContextAwareRequest request) {
return request.getContext() == null ? true :
BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true);
}
private OkHttpRibbonResponse executeWithRetry(OkHttpRibbonRequest request, LoadBalancedRetryPolicy retryPolicy,
RetryCallback<OkHttpRibbonResponse, Exception> callback) throws Exception {
RetryTemplate retryTemplate = new RetryTemplate();
......@@ -96,8 +107,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
if (retryListeners != null && retryListeners.length != 0) {
retryTemplate.setListeners(retryListeners);
}
boolean retryable = request.getContext() == null ? true :
BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true);
boolean retryable = isRequestRetryable(request);
retryTemplate.setRetryPolicy(retryPolicy == null || !retryable ? new NeverRetryPolicy()
: new RetryPolicy(request, retryPolicy, this, this.getClientName()));
return retryTemplate.execute(callback);
......@@ -154,7 +164,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null);
}
static class RetryPolicy extends FeignRetryPolicy {
static class RetryPolicy extends InterceptorRetryPolicy {
public RetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, ServiceInstanceChooser serviceInstanceChooser, String serviceName) {
super(request, policy, serviceInstanceChooser, serviceName);
}
......
......@@ -50,6 +50,10 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
protected final D delegate;
protected final IClientConfig config;
protected final ServerIntrospector serverIntrospector;
public boolean isClientRetryable(ContextAwareRequest request) {
return false;
}
@Deprecated
public AbstractLoadBalancingClient() {
......
......@@ -21,6 +21,8 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration;
import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse;
import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient;
import org.springframework.cloud.netflix.ribbon.support.ContextAwareRequest;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext;
......@@ -174,8 +176,16 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
final RequestContext context = RequestContext.getCurrentContext();
RQ request = createRequest();
RS response = this.client.executeWithLoadBalancer(request, config);
RS response;
boolean retryableClient = this.client instanceof AbstractLoadBalancingClient
&& ((AbstractLoadBalancingClient)this.client).isClientRetryable((ContextAwareRequest)request);
if (retryableClient) {
response = this.client.execute(request, config);
} else {
response = this.client.executeWithLoadBalancer(request, config);
}
context.set("ribbonResponse", response);
// Explicitly close the HttpResponse if the Hystrix command timed out to
......
......@@ -36,6 +36,7 @@ import org.springframework.http.HttpRequest;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient.RibbonServer;
import java.io.IOException;
import java.net.SocketException;
import java.util.Collections;
import java.util.Map;
......@@ -217,6 +218,37 @@ public class RibbonLoadBalancedRetryPolicyFactoryTests {
}
@Test
public void testCiruitRelatedExceptionsUpdateServerStats() throws Exception {
int sameServer = 3;
int nextServer = 3;
RibbonServer server = getRibbonServer();
IClientConfig config = mock(IClientConfig.class);
doReturn(sameServer).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt());
doReturn(nextServer).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt());
doReturn(false).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false));
doReturn(config).when(clientFactory).getClientConfig(eq(server.getServiceId()));
doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq(""));
clientFactory.getLoadBalancerContext(server.getServiceId()).setRetryHandler(new DefaultLoadBalancerRetryHandler(config));
RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server);
RibbonLoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory);
LoadBalancedRetryPolicy policy = factory.create(server.getServiceId(), client);
HttpRequest request = mock(HttpRequest.class);
LoadBalancedRetryContext context = spy(new LoadBalancedRetryContext(null, request));
doReturn(server).when(context).getServiceInstance();
policy.registerThrowable(context, new IOException());
verify(serverStats, times(0)).incrementSuccessiveConnectionFailureCount();
// Circuit Related should increment failure count
policy.registerThrowable(context, new SocketException());
verify(serverStats, times(1)).incrementSuccessiveConnectionFailureCount();
}
@Test
public void testRetryableStatusCodes() throws Exception {
int sameServer = 3;
int nextServer = 3;
......
......@@ -308,7 +308,7 @@ public class RibbonLoadBalancingHttpClientTests {
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(delegate, times(2)).execute(any(HttpUriRequest.class));
verify(lb, times(0)).chooseServer(eq(serviceName));
verify(lb, times(1)).chooseServer(eq(serviceName));
}
@Test
......@@ -342,7 +342,7 @@ public class RibbonLoadBalancingHttpClientTests {
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName));
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
}
......@@ -377,7 +377,7 @@ public class RibbonLoadBalancingHttpClientTests {
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(response, times(0)).close();
verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName));
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
}
......@@ -489,7 +489,7 @@ public class RibbonLoadBalancingHttpClientTests {
} catch(IOException e) {} finally {
verify(response, times(0)).close();
verify(delegate, times(1)).execute(any(HttpUriRequest.class));
verify(lb, times(0)).chooseServer(eq(serviceName));
verify(lb, times(1)).chooseServer(eq(serviceName));
}
}
......@@ -528,7 +528,7 @@ public class RibbonLoadBalancingHttpClientTests {
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(fourOFourResponse, times(1)).close();
verify(delegate, times(2)).execute(any(HttpUriRequest.class));
verify(lb, times(0)).chooseServer(eq(serviceName));
verify(lb, times(1)).chooseServer(eq(serviceName));
assertEquals(1, myBackOffPolicyFactory.getCount());
}
......@@ -564,7 +564,7 @@ public class RibbonLoadBalancingHttpClientTests {
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(response, times(0)).close();
verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName));
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
assertEquals(2, myRetryListeners.getOnError());
}
......@@ -601,7 +601,7 @@ public class RibbonLoadBalancingHttpClientTests {
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(response, times(0)).close();
verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName));
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
assertEquals(0, myRetryListeners.getOnError());
}
......@@ -670,7 +670,7 @@ public class RibbonLoadBalancingHttpClientTests {
RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(response, times(0)).close();
verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName));
verify(lb, times(2)).chooseServer(eq(serviceName));
assertEquals(2, myBackOffPolicyFactory.getCount());
}
......
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