Retries are not limited

[metadaddy]

Currently, if a B2 API returns a HTTP error status code that indicates a transient error, such as 429 or 503, there is no limit to the number of retries that will be attempted. The app calling the API will hang until the operation returns a non-transient status such as 200 or 404.

The issue is in the withBackoff() function:

func withBackoff(ctx context.Context, ri beRootInterface, f func() error) error {
	backoff := 500 * time.Millisecond
	for {
		err := f()
		if !ri.transient(err) {
			return err
		}
		bo := ri.backoff(err)
		if bo > 0 {
			backoff = bo
		} else {
			backoff = getBackoff(backoff)
		}
		select {
		case <-ctx.Done():
			return ctx.Err()
		case <-after(backoff):
		}
	}
}

Notice that the for loop has no terminating condition. There needs to be a configurable maximum number of retries so that operations can fail if the service is unavailable.

Note: I have looked at fixing this in the past. At the 'micro' level, it is straightforward to make this function do the right thing. The issues seem to arise in the Writer with multipart uploads, where multiple parts can be in flight concurrently.

[metadaddy]

I just pushed #19 - it contains a test function, TestWriterLimitNewUploadAttempts, that I wrote. It may be useful.

[mlech-reef]

Hi. I just started looking into it, wanted to clarify few things before I start making any changes in the code.

There needs to be a configurable maximum number of retries so that operations can fail if the service is unavailable.

What would be the desired default value here?

Note: I have looked at fixing this in the past. At the 'micro' level, it is straightforward to make this function do the right thing. The issues seem to arise in the Writer with multipart uploads, where multiple parts can be in flight concurrently.

I don't understand why concurrently is an issue here. If we have e.g. maxRetries flag somewhere, and it has value of e.g. 3, then each concurrent thread will retry 3 time before it fails. Not like that?

Thank you for clarification.

Best regards,
Maciek

[metadaddy]

Hi @mlech-reef -

There needs to be a configurable maximum number of retries so that operations can fail if the service is unavailable.

What would be the desired default value here?

Looking at the B2 SDK for Python, upload/download operations are retried 20 times, while other operations are retried 5 times.

I don't understand why concurrently is an issue here. If we have e.g. maxRetries flag somewhere, and it has value of e.g. 3, then each concurrent thread will retry 3 time before it fails. Not lie that?

My recollection is that, when an upload failed after 3 retries, I was trying to cancel all the other 'in-flight' uploads, to 'fail fast'. I was probably making it too complicated!