Skip to content

Commit c3c66de

Browse files
authored
add ServiceClientImpl retry on 401 (#18771)
1 parent 067ffe6 commit c3c66de

File tree

2 files changed

+19
-1
lines changed

2 files changed

+19
-1
lines changed

server/src/main/java/org/apache/druid/rpc/StandardRetryPolicy.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ public boolean retryHttpResponse(final HttpResponse response)
141141
return code == HttpResponseStatus.BAD_GATEWAY.getCode()
142142
|| code == HttpResponseStatus.SERVICE_UNAVAILABLE.getCode()
143143
|| code == HttpResponseStatus.GATEWAY_TIMEOUT.getCode()
144-
144+
// 401 can happen from things like expiration and might be retryable
145+
|| code == HttpResponseStatus.UNAUTHORIZED.getCode()
145146
// Technically shouldn't retry this last one, but servers sometimes return HTTP 500 for retryable errors.
146147
|| code == HttpResponseStatus.INTERNAL_SERVER_ERROR.getCode();
147148
}

server/src/test/java/org/apache/druid/rpc/ServiceClientImplTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,23 @@ private <T> OngoingStubbing<ListenableFuture<Either<StringFullResponseHolder, T>
715715
);
716716
}
717717

718+
@Test
719+
public void test_request_authErrorRetry() throws Exception
720+
{
721+
final RequestBuilder requestBuilder = new RequestBuilder(HttpMethod.GET, "/foo");
722+
final ImmutableMap<String, String> expectedResponseObject = ImmutableMap.of("foo", "bar");
723+
724+
// Unauthorized response from SERVER1, then OK response.
725+
stubLocatorCall(locations(SERVER1, SERVER2));
726+
expectHttpCall(requestBuilder, SERVER1)
727+
.thenReturn(errorResponse(HttpResponseStatus.UNAUTHORIZED, null, "unauthorized"))
728+
.thenReturn(valueResponse(expectedResponseObject));
729+
730+
serviceClient = makeServiceClient(StandardRetryPolicy.unlimited());
731+
final Map<String, String> response = doRequest(serviceClient, requestBuilder);
732+
Assert.assertEquals(expectedResponseObject, response);
733+
}
734+
718735
private void stubLocatorCall(final ServiceLocations locations)
719736
{
720737
stubLocatorCall(Futures.immediateFuture(locations));

0 commit comments

Comments
 (0)