Follow-up from "Replace time.Sleep with a cancelable timer inside the cache retriever"

The following discussion from !418 (closed) should be addressed:

• @ash2k started a discussion: (+1 comment)

  ~bug: We are leaking goroutines here:

  1. resolveWithBackoff() starts running
  2. ctx.Done() is selected
  3. resolveWithBackoff() does the job but then it does not have space in the channel (response := make(chan api.Lookup)) to put the result into so it's getting stuck forever and the goroutine and memory are leaked.

  I wonder why do we even need that extra goroutine? Looking and following the code, we use the context in the HTTP client and there is no other blocking operations. So I think this can be simplified:

  • Retrieve() should take a context.Context directly - as was suggested here !194 (comment 255169958) in the original MR. That gives the caller a possibility to cancel or set timeout (or both!) on the context.
  • resolveWithBackoff() can be made a synchronous method as there is no need for a goroutine.
  • select can be removed.
  • resolveWithBackoff() can be merged into Retrieve() as there is nothing apart from the call.

  The remaining thing is the time.Sleep that should be made (in my opinion...) interruptible to make sure the method unblocks asap.

  This is what I'd do :)