Fix retryOnRenew classes

These classes should not close the release the payload as they are not
reading it.

- fix swift
- fix openstack-swift v1 and v2
- fix RetryOnRenewTest for v1 and v2
This commit is contained in:
Andrea Turli 2016-10-26 16:55:21 +02:00
parent a843196bf6
commit 7167b473b4
5 changed files with 100 additions and 135 deletions

View File

@ -16,8 +16,6 @@
*/
package org.jclouds.openstack.keystone.v2_0.handlers;
import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
import java.util.concurrent.TimeUnit;
import javax.annotation.Resource;
@ -79,54 +77,45 @@ public class RetryOnRenew implements HttpRetryHandler {
@Override
public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) {
boolean retry = false; // default
try {
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
closeClientButKeepContentStream(response);
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
}
break;
case 408:
return backoffHandler.shouldRetryRequest(command, response);
}
return retry;
} finally {
// If the request is failed and is not going to be retried, the
// ErrorHandler will be invoked and it might need to read the payload.
// For some kind of payload sources, such as the OkHttp Source, if the
// payload is released, the upcoming operations will fail.
closeClientButKeepContentStream(response);
}
break;
case 408:
return backoffHandler.shouldRetryRequest(command, response);
}
return retry;
}
}

View File

@ -111,8 +111,6 @@ public class RetryOnRenewTest {
LoadingCache<Credentials, Access> cache = createMock(LoadingCache.class);
BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class);
expect(response.getPayload()).andReturn(Payloads.newStringPayload(
"The server has waited too long for the request to be sent by the client.")).times(3);
expect(backoffHandler.shouldRetryRequest(command, response)).andReturn(true).once();
expect(response.getStatusCode()).andReturn(408).once();

View File

@ -16,9 +16,6 @@
*/
package org.jclouds.openstack.handlers;
import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
import static org.jclouds.http.HttpUtils.releasePayload;
import java.util.concurrent.TimeUnit;
import javax.annotation.Resource;
@ -75,49 +72,43 @@ public class RetryOnRenew implements HttpRetryHandler {
@Override
public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) {
boolean retry = false; // default
try {
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
closeClientButKeepContentStream(response);
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
}
break;
}
return retry;
} finally {
releasePayload(response);
}
break;
}
return retry;
}
}

View File

@ -16,9 +16,6 @@
*/
package org.jclouds.openstack.keystone.v1_1.handlers;
import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
import static org.jclouds.http.HttpUtils.releasePayload;
import java.util.concurrent.TimeUnit;
import javax.annotation.Resource;
@ -80,51 +77,45 @@ public class RetryOnRenew implements HttpRetryHandler {
@Override
public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) {
boolean retry = false; // default
try {
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
closeClientButKeepContentStream(response);
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
&& headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
retry = false;
} else {
// This is not an authentication request returning 401
// Check if we already had seen this request
Integer count = retryCountMap.getIfPresent(command);
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
if (count == null) {
// First time this non-authentication request failed
logger.debug("invalidating authentication token - first time for %s", command);
retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
retry = true;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// This request has failed before
if (count + 1 >= NUM_RETRIES) {
logger.debug("too many 401s - giving up after: %s for %s", count, command);
retry = false;
} else {
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
// Retry just in case
logger.debug("invalidating authentication token - retry %s for %s", count, command);
retryCountMap.put(command, count + 1);
// Wait between retries
authenticationResponseCache.invalidateAll();
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
retry = true;
}
}
break;
case 408:
return backoffHandler.shouldRetryRequest(command, response);
}
return retry;
} finally {
releasePayload(response);
}
break;
case 408:
return backoffHandler.shouldRetryRequest(command, response);
}
return retry;
}
}

View File

@ -110,14 +110,11 @@ public class RetryOnRenewTest {
@Test
public void test408ShouldRetry() {
HttpCommand command = createMock(HttpCommand.class);
HttpRequest request = createMock(HttpRequest.class);
HttpResponse response = createMock(HttpResponse.class);
@SuppressWarnings("unchecked")
LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class);
BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class);
expect(response.getPayload()).andReturn(Payloads.newStringPayload(
"The server has waited too long for the request to be sent by the client.")).times(2);
expect(backoffHandler.shouldRetryRequest(command, response)).andReturn(true).once();
expect(response.getStatusCode()).andReturn(408).once();
@ -145,7 +142,6 @@ public class RetryOnRenewTest {
LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class);
BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class);
expect(response.getPayload()).andReturn(Payloads.newStringPayload("")).times(2);
expect(response.getStatusCode()).andReturn(404).once();
replay(command);