Periodically delete jobs in Mettle api server

[Hamishpk]

At times of high load the current method of scheduling actions for deletion using individual goroutines is causing connection/context deadline exceeded errors.

This change hopes to improve this by managing the deletion of jobs through one routine. Every 10 minutes the server will loop through all the jobs and delete it if:

• The job is complete and over the retention time (5 minutes)
• The job is incomplete, has no listeners and within the expiry time (60 minutes)
• The job is incomplete and outside of two times the expiry time (120 minutes)

I remember we had some nasty bugs a few years ago around this so I've added a bunch of tests for shouldDeleteJobs. I think this should cover all our cases but am slightly worried an empty job (done==false, LastUpdate == beginning of time) would be deleted. We never set an empty job so shouldn't be an issue but could be convinced to add a special case for this)

[fische]

I'm not sure about this approach; at the moment there's a data race that would cause a panic, as you're accessing the jobs map without locking the mutex, but I think locking the mutex to go through the list of jobs might be a bit too expensive. I think it would be nicer to do this with a channel. By that, I mean we send all jobs that we want to expire (the old go s.deleteJob(key, j)) on the channel and the background routine appends those to an internal list. That way it can then assume the first item in that list is the next to get deleted and the implementation would look like this:

for {
  select {
  case job := <-channel:
    buffer = append(buffer, job)
  case <-time.After(time.Until(buffer[0].ExpirationTime)):
    // delete job from map
    buffer = buffer[1:]
  }
}

This is of course oversimplified (ie we shouldnt start a new Timer every time but just reuse the same one) but it's the general idea. What do you think?

[peterebden]

I wonder if it might be simpler to just lock around the whole map iteration. I would hope it doesn't take too long to iterate it - we don't have to ask a complex question about each item.

Alternatively we could use a sync.Map but probably easier not to start down that rabbit hole...

[Hamishpk]

After an in-person chat with @fische it was decided to go for @peterebden suggestion of locking the whole map. I have added a metric to track the time of this and will refactor if it proves to be troublesome