Add requests backoff mechanism

What does this MR do?

Adds a requests backoff mechanism, that makes handling 429 response more intelligent.

Why was this MR needed?

Please read #2377 (closed) for a reference

Are there points in the code the reviewer needs to double check?

Does this MR meet the acceptance criteria?

• Documentation created/updated
• Tests
  • Added for this feature/bug
  • All builds are passing
• Branch has no merge conflicts with master (if you do - rebase it please)

What are the relevant issue numbers?

Closes #2377 (closed)

@nick.thomas @ayufan What do you think about this approach?

Backoff mechanism in network/client.go is added in the requests layer, so it'll be easy to handle 429 properly also for other requests if it'll be needed in the future.

In case of jobs/request request, change in network/client.go will delay a request, and request_concurrency setting will make sure that new requests are not made until old are finished.

@tmaczukin I wonder if we should support the Retry-After header mentioned here: https://tools.ietf.org/html/rfc6585#section-4

We already use it in the GitLab frontend to allow the server administrator to dynamically adjust the polling interval.

Using a standard exponential backoff if the header isn't present seems sensible, though.

assigned to @tmaczukin

resolved all discussions

mentioned in issue #2429

@nick.thomas

I wonder if we should support the Retry-After header mentioned here: https://tools.ietf.org/html/rfc6585#section-4

429 for Runners requests is currently sent only for /api/v4/jobs/request endpoint and can be send by GitLab Workhorse (in every GitLab installation which has configured requests queuing) and by HaProxy (in case of GitLab.com). AFAIR none of these is sending the Retry-After header. The idea to support of this header is good, but it will be not used at all at this moment. We can spend some time and implement it now or do this when (and if) we decide to use this address in our API. I would prefer to go with the second way.

I've added fixes including your comments. Can you review again?

assigned to @nick.thomas

@tmaczukin the code looks good to me now, just a question about the approach.

If you're confident in the timings and how this will stack up in real use, feel free to merge

assigned to @tmaczukin

resolved all discussions

@nick.thomas Can you approve the MR?

assigned to @nick.thomas

mentioned in issue #2449 (closed)

resolved all discussions

@ayufan MR is updated

assigned to @ayufan

resolved all discussions

assigned to @tmaczukin

changed milestone to %9.3

@ayufan I've updated the MR.

assigned to @ayufan

@nick.thomas WDYT?

LGTM

approved this merge request

merged