Skip to content

Commit

Permalink
Add new retry policy adjustment method (#1824)
Browse files Browse the repository at this point in the history
* Add new retry policy adjustment method for when we have a response object

* Fix test

* Actually fix test

* Break out to separate method

* Fix tests
  • Loading branch information
jguerra authored Sep 24, 2024
1 parent b008f90 commit fc74635
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,6 @@
import io.netty.util.concurrent.Promise;
import io.perfmark.PerfMark;
import io.perfmark.TaskCloseable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import java.io.UnsupportedEncodingException;
import java.net.InetAddress;
import java.net.URLDecoder;
Expand All @@ -104,6 +100,9 @@
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Not thread safe! New instance of this class is created per HTTP/1.1 request proxied to the origin but NOT for each
Expand Down Expand Up @@ -738,8 +737,8 @@ protected boolean isRetryable(final ErrorType err) {
if ((err == OutboundErrorType.RESET_CONNECTION)
|| (err == OutboundErrorType.CONNECT_ERROR)
|| (err == OutboundErrorType.READ_TIMEOUT
&& IDEMPOTENT_HTTP_METHODS.contains(
zuulRequest.getMethod().toUpperCase()))) {
&& IDEMPOTENT_HTTP_METHODS.contains(
zuulRequest.getMethod().toUpperCase()))) {
return isRequestReplayable();
}
return false;
Expand Down Expand Up @@ -908,6 +907,7 @@ protected void handleOriginNonSuccessResponse(final HttpResponse originResponse,

boolean retryable5xxResponse = isRetryable5xxResponse(zuulRequest, originResponse);
if (retryable5xxResponse) {
origin.originRetryPolicyAdjustmentIfNeeded(zuulRequest, originResponse);
origin.adjustRetryPolicyIfNeeded(zuulRequest);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.netflix.zuul.niws.RequestAttempt;
import com.netflix.zuul.passport.CurrentPassport;
import io.netty.channel.EventLoop;
import io.netty.handler.codec.http.HttpResponse;
import io.netty.util.concurrent.Promise;
import java.net.InetAddress;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -77,4 +78,6 @@ RequestAttempt newRequestAttempt(
IClientConfig getClientConfig();

Registry getSpectatorRegistry();

default void originRetryPolicyAdjustmentIfNeeded(HttpRequestMessage zuulReq, HttpResponse nettyResponse) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
Expand Down Expand Up @@ -156,7 +157,8 @@ void retryWhenNoAdjustment() {
createResponse(HttpResponseStatus.SERVICE_UNAVAILABLE);

proxyEndpoint.handleOriginNonSuccessResponse(response, createDiscoveryResult());
verify(nettyOrigin).adjustRetryPolicyIfNeeded(request);
verify(nettyOrigin).adjustRetryPolicyIfNeeded(eq(request));
verify(nettyOrigin).originRetryPolicyAdjustmentIfNeeded(request, response);
verify(nettyOrigin).connectToOrigin(any(), any(), anyInt(), any(), any(), any());
}

Expand All @@ -174,6 +176,7 @@ void noRetryAdjustmentOnNonRetriableStatusCode() {
createResponse(HttpResponseStatus.BAD_REQUEST);
proxyEndpoint.handleOriginNonSuccessResponse(response, createDiscoveryResult());
verify(nettyOrigin, never()).adjustRetryPolicyIfNeeded(request);
verify(nettyOrigin, never()).originRetryPolicyAdjustmentIfNeeded(request, response);
validateNoRetry();
}

Expand Down Expand Up @@ -202,6 +205,7 @@ public void onErrorFromOriginNoRetryOnNonRetriableError() {

proxyEndpoint.errorFromOrigin(new RuntimeException());
verify(nettyOrigin, never()).adjustRetryPolicyIfNeeded(request);
verify(nettyOrigin, never()).originRetryPolicyAdjustmentIfNeeded(request, response);
validateNoRetry();
}

Expand Down

0 comments on commit fc74635

Please sign in to comment.