1620350 - go client should support configuring and disabling retries

Status: RESOLVED FIXED

(Taskcluster :: Workers, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

We disable retries in other contexts for various reasons. For example, in hooks, we retry task creation in a loop, so there's no need for an individual queue.createTask to retry internally -- and in fact the long delay in the exponential backoff is undesirable.

We should have the same flexibility in the go client.

Comment 1

[Pete Moore [:pmoore][:pete]]

Currently the call is made here using the default backoff settings.

It looks like currently httpbackoff assumes backoff settings are backoff.ExponentialBackOff settings. We should update that to use the backoff.Backoff interface rather than the backoff.ExponentialBackOff type explicitly, so that any of the provided backoff schemes can be used, such as backoff.StopBackOff (meaning no retries).

I suspect the common case is having per-client backoff settings rather than per-API-call backoff settings, so after updating httpbackoff as described above, we could provide a means to set the backoff settings by adding the settings at a client level. Calling code that wishes to disable backoffs could then look like this:

	queue := tcqueue.New(c.Credentials(), c.RootURL)
	queue.HTTPBackOffClient = &httpbackoff.Client{
		BackOffSettings: backoff.StopBackOff{},
	}

To achieve this, we would need to add HTTPBackOffClient *httpbackoff.Client to the tcclient.Client type here. Note the use of a pointer type, to make it easy to detect whether it is configured (non-nil) or not.

Then the line here would need to adapted to use the value if specified, e.g. something like:

	httpBackOffClient := defaultBackoff
	if client.HTTPBackOffClient != nil {
		httpBackOffClient = *client.HTTPBackOffClient
	}
	callSummary.HTTPResponse, callSummary.Attempts, err = httpBackOffClient.Retry(httpCall)

Comment 2

[Aushio]

Hello,

I'm a new graduate looking to gain some experience working on open-source projects and continue to learn and develop my skills.

Is this bug still available to be worked on?

While my current experience working with Go is fairly limited, I have extensive experience throughout my education working in C and have been doing some small projects over the past few weeks to gain some familiarity with the language.

Regards,

• Aushio

Comment 3

[Pete Moore [:pmoore][:pete]]

Yes, sure! We'd love any help we can get. Many thanks Aushio!

Comment 4

[Pete Moore [:pmoore][:pete]]

Oh, it looks like maybe this has already been implemented: https://github.com/taskcluster/taskcluster/tree/main/clients/client-go#specifying-exponential-backoff-settings-for-http-request-retries ...

Comment 5

[Pete Moore [:pmoore][:pete]]

Indeed, looks like it slipped into: https://github.com/taskcluster/taskcluster/pull/4493

One less bug, yay! :-)

Comment 6

[Pete Moore [:pmoore][:pete]]

Attachment: GitHub Pull Request for taskcluster monorepo