Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpStorageRpc#getCurrentUploadOffset uses 0 as error code in StorageException #687

Closed
tomcham opened this issue Jan 16, 2021 · 3 comments
Closed
Assignees
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tomcham
Copy link

tomcham commented Jan 16, 2021

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Please run down the following list and make sure you've tried the usual "quick fixes":

If you are still having issues, please include as much information as possible:

Environment details

  1. Specify the API at the beginning of the title. For example, "BigQuery: ...").
    General, Core, and Other are also allowed as types
  2. OS type and version:
  3. Java version: 1.8.x
  4. storage version(s): 1.113.9

Steps to reproduce

Code example

// example

Stack trace

c.g.c.s.StorageException: Not sure what occurred. Here's debugging information:
Response:
com.google.api.client.http.HttpResponseException: 503 Service Unavailable
PUT https://storage.googleapis.com/upload/storage/v1/b/.../o?name=...&uploadType=resumable&upload_id=...


	at c.g.c.s.s.v.HttpStorageRpc.getCurrentUploadOffset(HttpStorageRpc.java:790) ~[google-cloud-storage-1.113.9.jar:1.113.9]
	at c.g.c.s.BlobWriteChannel$1.run(BlobWriteChannel.java:133)
	at j.u.c.Executors$RunnableAdapter.call(Executors.java:511) ~[na:1.8.0_181]
	at c.g.a.g.r.DirectRetryingExecutor.submit(DirectRetryingExecutor.java:105) ~[gax-1.60.1.jar:1.60.1]
	at c.g.c.RetryHelper.run(RetryHelper.java:76) ~[google-cloud-core-1.94.0.jar:1.94.0]
	at c.g.c.RetryHelper.runWithRetries(RetryHelper.java:50)
	at c.g.c.s.BlobWriteChannel.flushBuffer(BlobWriteChannel.java:70) ~[google-cloud-storage-1.113.9.jar:1.113.9]
	at c.g.c.BaseWriteChannel.close(BaseWriteChannel.java:151) ~[google-cloud-core-1.94.0.jar:1.94.0]
	at j.n.c.Channels$1.close(Channels.java:178) ~[na:1.8.0_181]

External references such as API reference guides

Any additional information below

I am using https://github.com/googleapis/java-storage/blob/v1.113.9/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java#L2673 to stream uploads.

Occasionally 503 response code is returned when executing https://github.com/googleapis/java-storage/blob/v1.113.9/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java#L752 and 0 is used as the error code when throwing a StorageException. If I understand correctly the error code is used to determine when to retry and 0 is not part of the retry-able error codes (https://github.com/googleapis/java-storage/blob/v1.113.9/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java#L40), but in this case we should retry 503s.

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage API. label Jan 16, 2021
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Jan 16, 2021
@frankyn frankyn added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. triage me I really want to be triaged. labels Jan 21, 2021
@tomcham
Copy link
Author

tomcham commented Feb 18, 2021

Hello, @frankyn any estimated time when this will be fixed?

@frankyn frankyn self-assigned this Feb 18, 2021
@frankyn
Copy link
Contributor

frankyn commented Feb 18, 2021

Hi @tomcham,

Thanks for the ping, the error code 0 is to prevent retries in this case, but looks like we need to add retries to the getCurrentUploadOffset which was not how it's implemented.

Thanks for raising this issue, I'll work to get this fixed within the next week.

gcf-merge-on-green bot pushed a commit that referenced this issue Feb 26, 2021
…#726)

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)


I added three things in this PR:
1. Retries around getting remote offset from GCS
2. Retrying last chunk failures correctly. 
3. Updated documentation for the retry cases


Fixes #709 #687 ☕️
@frankyn
Copy link
Contributor

frankyn commented Mar 2, 2021

The fix was released in version 1.113.12

@frankyn frankyn closed this as completed Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants