Storage Retry Logic

Hi there. I'm using com.google.cloud.storage.Storage.writer() method of v0.8.0 of the google-cloud lib. I'm getting "503 backend error" from the GCS service when trying to upload files (stack trace below). I expect the retry logic in com.google.cloud.storage.BlobWriteChannel to retry the chunk, but it doesn't do that.

  @Override
  protected void flushBuffer(final int length, final boolean last) {
    try {
      runWithRetries(callable(new Runnable() {
        @Override
        public void run() {
          getOptions().getRpc().write(getUploadId(), getBuffer(), 0, getPosition(), length, last);
        }
      }), getOptions().getRetryParams(), StorageImpl.EXCEPTION_HANDLER, getOptions().getClock());
    } catch (RetryHelper.RetryHelperException e) {
      throw StorageException.translateAndThrow(e);
    }
  }

The problem seems to lie in the logic for converting the com.google.api.client.http.HttpResponseException to a com.google.cloud.storage.StorageException

public BaseServiceException(IOException exception, boolean idempotent) {
    super(message(exception), exception);
    int code = UNKNOWN_CODE;
    String reason = null;
    String location = null;
    String debugInfo = null;
    Boolean retryable = null;
    if (exception instanceof GoogleJsonResponseException) {
      GoogleJsonError jsonError = ((GoogleJsonResponseException) exception).getDetails();
      if (jsonError != null) {
        Error error = new Error(jsonError.getCode(), reason(jsonError));
        code = error.code;
        reason = error.reason;
        retryable = isRetryable(idempotent, error);
        if (reason != null) {
          GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0);
          location = errorInfo.getLocation();
          debugInfo = (String) errorInfo.get("debugInfo");
        }
      } else {
        code = ((GoogleJsonResponseException) exception).getStatusCode();
      }
    }
    this.retryable = MoreObjects.firstNonNull(retryable, isRetryable(idempotent, exception));
    this.code = code;
    this.reason = reason;
    this.idempotent = idempotent;
    this.location = location;
    this.debugInfo = debugInfo;
  }

Since the HttpResponseException is not an instanceof GoogleJsonResponseException, we end up with a code of 0, which the retry logic sees as a non-retryable exception. Here's a code snippet to easily reproduce the issue:

HttpResponseException httpEx = new HttpResponseException.Builder(503, "Service Unavailable", new HttpHeaders()).build();
StorageException storageEx = new StorageException(httpEx);
System.out.println(storageEx.getCode());
System.out.println(storageEx.getMessage());

Furthermore, the stacktrace I end up with doesn't indicate that the code and message of the ServiceException weren't set (I think the toString() method can be improved to show this info).

Exception in thread "main" com.google.cloud.storage.StorageException: 503 Service Unavailable
{
"error": {
"errors": [
{
"domain": "global",
"reason": "backendError",
"message": "Backend Error"
}
],
"code": 503,
"message": "Backend Error"
}
}

at com.google.cloud.storage.spi.DefaultStorageRpc.translate(DefaultStorageRpc.java:202)
at com.google.cloud.storage.spi.DefaultStorageRpc.write(DefaultStorageRpc.java:582)
at com.google.cloud.storage.BlobWriteChannel$1.run(BlobWriteChannel.java:50)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
at com.google.cloud.RetryHelper.doRetry(RetryHelper.java:179)
at com.google.cloud.RetryHelper.runWithRetries(RetryHelper.java:244)
at com.google.cloud.storage.BlobWriteChannel.flushBuffer(BlobWriteChannel.java:47)
at com.google.cloud.BaseWriteChannel.close(BaseWriteChannel.java:199)
at com.spins.google.api.CloudStorage.upload(CloudStorage.java:338)
at com.spins.google.api.CloudStorage.upload(CloudStorage.java:325)
at com.spins.google.api.CloudStorage.upload(CloudStorage.java:315)
at Random.main(Random.java:69)

Caused by: com.google.api.client.http.HttpResponseException: 503 Service Unavailable
{
"error": {
"errors": [
{
"domain": "global",
"reason": "backendError",
"message": "Backend Error"
}
],
"code": 503,
"message": "Backend Error"
}
}

at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:1061)
at com.google.cloud.storage.spi.DefaultStorageRpc.write(DefaultStorageRpc.java:564)
... 10 more

@mziccard

@mziccard - you helped me out before with a similar issue: #1135. Any chance you could take a look at this?

@Dima1224

@Dima1224 , @mziccard is no longer active on this project. I am marking this Blocking GA - we'll get it fixed ASAP; it seems quite important to retry 503 errors.

Thanks! I should mention we see the same issue with SocketTimeoutException once in a while.

Hey guys, any update on this?

@Dima1224

@Dima1224 I'm looking into it. This is on the top of my to-do this week. Will give you update soon :)

@Dima1224

@Dima1224 I definitely agree with you in that we didn't properly handle constructing StorageException from HttpResponseException - we didn't register the right error code in StorageException, which in turn fails the Retry logic. However, on a second look of the error message you posted, what puzzles me is the exception looks like GoogleJsonResponseException to me, since it has domain and reason fields. If that's case, then we are actually having a different problem, i.e., how the exception upcasting happened. Since if it were GoogleJsonResponseException, the Retry logic should work on it. What do you think ?

@neozwu

Hey @neozwu - this is exactly the problem I'm referring to in the second half of the ticket:

Furthermore, the stacktrace I end up with doesn't indicate that the code and message of the ServiceException weren't set (I think the toString() method can be improved to show this info).

The StorageException uses the message from the HttpResponseException verbatim without adding any info of it's own. So on the surface it looks like the fields of the StorageException were set correctly when in reality they were not. I think this can be addressed in the BaseServiceException class.

fix retry logic on HttpResponseException (googleapis#1545)

@garrettjonesgoogle

Hey @garrettjonesgoogle, I was actually wondering about a release date for this issue. 1735 doesn't concern me as much.

@Dima1224

@Dima1224 I misinterpreted your message, I'm very sorry about that! We intend on doing a new release this week. Do be aware that the next release will have some breaking changes, though.

@garrettjonesgoogle

Awesome, thanks @garrettjonesgoogle! Is there any way for me to sign up for release notifications? Might be a github feature I'm not aware of?

Subscribe to this atom feed: https://github.com/GoogleCloudPlatform/google-cloud-java/releases.atom

Thanks! I notice all releases after 0.8.0 are named *-alpha. Does this mean I shouldn't hang dependencies on them or is this the way you'll be naming all future releases with some components still in alpha?

The google-cloud artifact is marked as alpha because it pulls in some components that are alpha and some that are beta, so it didn't seem right to promise beta for everything. Basically, you have to be aware of the quality level of the individual clients that you use. Alternatively, you can depend directly on the individual clients that you need, which will be marked at their corresponding release level (alpha or beta).

@garrettjonesgoogle

One more question @garrettjonesgoogle. I've upgraded to 0.9.4-alpha of the storage lib and am now seeing yet another case of an exception which should be retried but is not (javax.net.ssl.SSLHandshakeException). Should I create another ticket for this?

@Dima1224

@Dima1224 yes, go ahead and create another issue.

This is a new root cause
Newer issues will be more visible by default

chore(deps): update dependency com.google.cloud:google-cloud-logging …

chore: update ignore paths in renovate config (googleapis#1945) (goog…

deps: update dependency com.google.cloud:google-cloud-storage to v2.4…

Navigation Menu

Repository navigation

Storage Retry Logic #1545

9 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Storage Retry Logic #1545

Description

Activity

Dima1224 commented on Jan 20, 2017

garrettjonesgoogle commented on Jan 26, 2017

Dima1224 commented on Jan 26, 2017

Dima1224 commented on Feb 10, 2017

neozwu commented on Feb 13, 2017

neozwu commented on Feb 21, 2017

Dima1224 commented on Feb 22, 2017

9 remaining items

Dima1224 commented on Mar 13, 2017

garrettjonesgoogle commented on Mar 13, 2017

Dima1224 commented on Mar 13, 2017

garrettjonesgoogle commented on Mar 13, 2017

Dima1224 commented on Mar 13, 2017

garrettjonesgoogle commented on Mar 13, 2017

Dima1224 commented on Mar 16, 2017

garrettjonesgoogle commented on Mar 16, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions