Skip to content

Commit

Permalink
chore(retry): implement new retry exception handlers (#1058)
Browse files Browse the repository at this point in the history 
* add idempotent handler
* add idempotent resumable write handler
* add non-idempotent handler

test count summary shift after this change
failure -> success: 55
  error -> success: 24
success -> failure: 26
  • Loading branch information
@BenWhitehead
BenWhitehead committed Oct 1, 2021
1 parent 8717cd6 commit 35cb2f7
Show file tree
Hide file tree
Showing 6 changed files with 422 additions and 39 deletions.
3 changes: 2 additions & 1 deletion google-cloud-storage/src/main/java/com/google/cloud/storage/CopyWriter.java
View file
Expand Up @@ -97,10 +97,11 @@ public long getTotalBytesCopied() {
*/
public void copyChunk() {
if (!isDone()) {
RewriteRequest rewriteRequest = rewriteResponse.rewriteRequest;
this.rewriteResponse =
Retrying.run(
serviceOptions,
serviceOptions.getRetryAlgorithmManager().getForObjectsCopy(),
serviceOptions.getRetryAlgorithmManager().getForObjectsRewrite(rewriteRequest),
() -> storageRpc.continueRewrite(rewriteResponse),
Function.identity());
}
Expand Down
7 changes: 1 addition & 6 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/LegacyRetryAlgorithmManager.java
View file
Expand Up @@ -219,14 +219,9 @@ public ExceptionHandler getForObjectsRewrite(RewriteRequest pb) {
return BaseService.EXCEPTION_HANDLER;
}

@Override
public ExceptionHandler getForObjectsCopy() {
return BaseService.EXCEPTION_HANDLER;
}

@Override
public ExceptionHandler getForObjectsCompose(
List<StorageObject> sources, StorageObject target, Map<StorageRpc.Option, ?> targetOptions) {
List<StorageObject> sources, StorageObject target, Map<StorageRpc.Option, ?> optionsMap) {
return BaseService.EXCEPTION_HANDLER;
}

Expand Down
340 changes: 338 additions & 2 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/NewRetryAlgorithmManager.java
View file
Expand Up @@ -16,5 +16,341 @@

package com.google.cloud.storage;

final class NewRetryAlgorithmManager extends LegacyRetryAlgorithmManager
implements RetryAlgorithmManager {}
import com.google.api.client.googleapis.json.GoogleJsonResponseException;
import com.google.api.services.storage.model.Bucket;
import com.google.api.services.storage.model.BucketAccessControl;
import com.google.api.services.storage.model.HmacKeyMetadata;
import com.google.api.services.storage.model.ObjectAccessControl;
import com.google.api.services.storage.model.Policy;
import com.google.api.services.storage.model.StorageObject;
import com.google.cloud.BaseServiceException;
import com.google.cloud.ExceptionHandler;
import com.google.cloud.ExceptionHandler.Interceptor;
import com.google.cloud.storage.spi.v1.StorageRpc;
import com.google.cloud.storage.spi.v1.StorageRpc.RewriteRequest;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
import java.util.List;
import java.util.Map;

final class NewRetryAlgorithmManager implements RetryAlgorithmManager {

private static final Interceptor INTERCEPTOR_IDEMPOTENT =
new InterceptorImpl(
true,
ImmutableSet.<Integer>builder()
.add(408)
.add(429)
.add(500)
.add(502)
.add(503)
.add(504)
.build());
private static final Interceptor INTERCEPTOR_IDEMPOTENT_RESUMABLE =
new InterceptorImpl(
true,
ImmutableSet.<Integer>builder().add(408).add(500).add(502).add(503).add(504).build());
private static final Interceptor INTERCEPTOR_NON_IDEMPOTENT =
new InterceptorImpl(false, ImmutableSet.<Integer>builder().build());

private static final ExceptionHandler IDEMPOTENT_HANDLER =
ExceptionHandler.newBuilder()
.retryOn(RuntimeException.class)
.addInterceptors(INTERCEPTOR_IDEMPOTENT)
.build();

private static final ExceptionHandler IDEMPOTENT_RESUMABLE_UPLOAD_HANDLER =
ExceptionHandler.newBuilder()
.retryOn(RuntimeException.class)
.addInterceptors(INTERCEPTOR_IDEMPOTENT_RESUMABLE)
.build();

private static final ExceptionHandler NON_IDEMPOTENT_HANDLER =
ExceptionHandler.newBuilder()
.retryOn(RuntimeException.class)
.addInterceptors(INTERCEPTOR_NON_IDEMPOTENT)
.build();

@Override
public ExceptionHandler getForBucketAclCreate(
BucketAccessControl pb, Map<StorageRpc.Option, ?> optionsMap) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketAclDelete(String pb, Map<StorageRpc.Option, ?> optionsMap) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketAclGet(String pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketAclUpdate(
BucketAccessControl pb, Map<StorageRpc.Option, ?> optionsMap) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketAclList(String pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsCreate(Bucket pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsDelete(Bucket pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsGet(Bucket pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsUpdate(Bucket pb, Map<StorageRpc.Option, ?> optionsMap) {
// TODO: Include etag when it is supported by the library
return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsList(Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsLockRetentionPolicy(
Bucket pb, Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsGetIamPolicy(
String bucket, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsSetIamPolicy(
String bucket, Policy pb, Map<StorageRpc.Option, ?> optionsMap) {
// TODO: Include etag when it is supported by the library
return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForBucketsTestIamPermissions(
String bucket, List<String> permissions, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForDefaultObjectAclCreate(ObjectAccessControl pb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForDefaultObjectAclDelete(String pb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForDefaultObjectAclGet(String pb) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForDefaultObjectAclUpdate(ObjectAccessControl pb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForDefaultObjectAclList(String pb) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForHmacKeyCreate(String pb, Map<StorageRpc.Option, ?> optionsMap) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForHmacKeyDelete(
HmacKeyMetadata pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForHmacKeyGet(String accessId, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForHmacKeyUpdate(
HmacKeyMetadata pb, Map<StorageRpc.Option, ?> optionsMap) {
// TODO: Include etag when it is supported by the library
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForHmacKeyList(Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectAclCreate(ObjectAccessControl aclPb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectAclDelete(
String bucket, String name, Long generation, String pb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectAclList(String bucket, String name, Long generation) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectAclGet(
String bucket, String name, Long generation, String pb) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectAclUpdate(ObjectAccessControl aclPb) {
return NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsCreate(
StorageObject pb, Map<StorageRpc.Option, ?> optionsMap) {
if (pb.getGeneration() != null && pb.getGeneration() == 0) {
return IDEMPOTENT_HANDLER;
}
return optionsMap.containsKey(StorageRpc.Option.IF_GENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsDelete(
StorageObject pb, Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_GENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsGet(StorageObject pb, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsUpdate(
StorageObject pb, Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsList(String bucket, Map<StorageRpc.Option, ?> optionsMap) {
return IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsRewrite(RewriteRequest pb) {
return (pb.sourceOptions.containsKey(StorageRpc.Option.IF_SOURCE_GENERATION_MATCH)
|| pb.sourceOptions.containsKey(StorageRpc.Option.IF_GENERATION_MATCH))
&& pb.targetOptions.containsKey(StorageRpc.Option.IF_GENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForObjectsCompose(
List<StorageObject> sources, StorageObject target, Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_METAGENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForResumableUploadSessionCreate(Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_GENERATION_MATCH)
? IDEMPOTENT_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForResumableUploadSessionWrite(Map<StorageRpc.Option, ?> optionsMap) {
return optionsMap.containsKey(StorageRpc.Option.IF_GENERATION_MATCH)
? IDEMPOTENT_RESUMABLE_UPLOAD_HANDLER
: NON_IDEMPOTENT_HANDLER;
}

@Override
public ExceptionHandler getForServiceAccountGet(String pb) {
return IDEMPOTENT_HANDLER;
}

private static class InterceptorImpl implements Interceptor {

private final boolean idempotent;
private final ImmutableSet<Integer> retryableCodes;

private InterceptorImpl(boolean idempotent, ImmutableSet<Integer> retryableCodes) {
this.idempotent = idempotent;
this.retryableCodes = retryableCodes;
}

@Override
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
return RetryResult.CONTINUE_EVALUATION;
}

@Override
public RetryResult beforeEval(Exception exception) {

// first check if an IO exception has been wrapped by a StorageException, fallback to
// a general BaseServiceException to check status code
if (exception instanceof StorageException) {
StorageException storageException = (StorageException) exception;
Throwable cause = storageException.getCause();
//noinspection StatementWithEmptyBody
if (cause instanceof GoogleJsonResponseException) {
// this is handled by the case for BaseServiceException below
} else if (cause instanceof IOException) {
IOException ioException = (IOException) cause;
return BaseServiceException.isRetryable(idempotent, ioException)
? RetryResult.RETRY
: RetryResult.NO_RETRY;
}
}

if (exception instanceof BaseServiceException) {
int code = ((BaseServiceException) exception).getCode();
if (retryableCodes.contains(code)) {
return RetryResult.RETRY;
} else {
return RetryResult.NO_RETRY;
}
}
return RetryResult.CONTINUE_EVALUATION;
}
}
}
4 changes: 1 addition & 3 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/RetryAlgorithmManager.java
View file
Expand Up @@ -105,10 +105,8 @@ ExceptionHandler getForBucketsTestIamPermissions(

ExceptionHandler getForObjectsRewrite(RewriteRequest pb);

ExceptionHandler getForObjectsCopy();

ExceptionHandler getForObjectsCompose(
List<StorageObject> sources, StorageObject target, Map<StorageRpc.Option, ?> targetOptions);
List<StorageObject> sources, StorageObject target, Map<StorageRpc.Option, ?> optionsMap);

ExceptionHandler getForResumableUploadSessionCreate(Map<StorageRpc.Option, ?> optionsMap);
/** Resumable upload has differing 429 handling */
Expand Down
2 changes: 1 addition & 1 deletion google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java
View file
Expand Up @@ -146,7 +146,7 @@ protected StorageRpc getStorageRpcV1() {
return (StorageRpc) getRpc();
}

protected RetryAlgorithmManager getRetryAlgorithmManager() {
RetryAlgorithmManager getRetryAlgorithmManager() {
return retryAlgorithmManager;
}

Expand Down

0 comments on commit 35cb2f7

Please sign in to comment.