Storage Retry Logic

[Dima1224]

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

[Dima1224]

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

[garrettjonesgoogle]

@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.

[Dima1224]

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

[Dima1224]

Hey guys, any update on this?

[neozwu]

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

[neozwu]

@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 ?

[Dima1224]

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.