gensupport: resumable uploads shouldn't use a hardcoded retryDeadline

[hphilipps]

There is a hardcoded retryDeadline of 32s for each chunk of a resumable upload which seems rather short for reliable uploads: https://github.com/googleapis/google-api-go-client/blob/master/internal/gensupport/resumable.go#L27

When doing many resumable uploads to GCS we often encounter 503 responses for some objects lasting for longer than 32s, which makes the whole upload for that object fail.

It would be great if we could just use the deadline of the context for timing out retries or make the retryDeadline an option.

[codyoss]

cc @tritone

[rogerhub]

I keep running into HTTP 502 errors on GCS writes, and it took an embarrasingly long time to figure out this was the root cause. The hardcoded 32 second timeout means that @tritone's recent work on improving GCS retries in googleapis/google-cloud-go#5210 is basically useless for me.

On my peasant residential internet connection, the default chunk size (16MiB) takes longer than 32 seconds to upload (especially considering concurrent writes, intentional network throttling, and the unfortunate realities of cable internet), so there's no time left for retries if the first attempt fails. What's worse is that since this value is hardcoded, I had to patch google-api-go-client in order to extend the timeout.

diff --git a/internal/gensupport/retry.go b/internal/gensupport/retry.go
index 4a4861b..8674bbb 100644
--- a/internal/gensupport/retry.go
+++ b/internal/gensupport/retry.go
@@ -21,7 +21,7 @@ type Backoff interface {
 // These are declared as global variables so that tests can overwrite them.
 var (
        // Per-chunk deadline for resumable uploads.
-       retryDeadline = 32 * time.Second
+       retryDeadline = 3600 * time.Second
        // Default backoff timer.
        backoff = func() Backoff {
                return &gax.Backoff{Initial: 100 * time.Millisecond}

Please consider increasing the default or making this configurable (or both). Thanks! :)

[tritone]

Thanks @rogerhub for the report, and apologies for dropping the ball on this issue earlier. I have a PR up at #1414 to resolve; this would have to be exposed on the manual client via a field on the Writer. Feel free to take a look!

[rogerhub]

Thanks for the quick action on this @tritone!