Smooth out spikes in rate of chunk flush ops

[bboreham]

Ingester chunk flushes run periodically, by default every minute.
Add a rate-limiter so we spread out calls to the DB across the period, avoiding spikes of intense activity which can slow down other operations such as incoming Push() calls.

Fixes #3171

Checklist

• NA Tests updated
• NA Documentation added
• CHANGELOG.md updated

[storyicon]

LGTM, this effectively evens out the pressure on the DB.

[bboreham]

TestIngesterSpreadFlush() is failing consistently in CI, though not on my local machine.
I will try to figure that out soon.

[pstibrany]

I think this interacts badly with flush on shutdown or via /flush handler. In these cases we want to flush as fast as possible.

Perhaps the call to i.flushRateLimiter.Wait should be done after dequeing and only if op.immediate is false? WDYT?

(Alternative – setting infinite rate for sweep with immediate flag wouldn't work, as rate would be recomputed on next call to sweepUsers)

[bboreham]

As I first wrote it (wait before dequeuing), there were some surprising behaviours.
For instance, if things were quiet, 50 goroutines would pass the rate-limiter wait and all block on the queue. Then when series are added to the queue they are picked up instantly and sent to the store, then each goroutine goes back round its loop and quite likely doesn't block on the ratelimiter once again, because as far as it is concerned nobody did anything for a while. So we still get "spiky" behaviour which I was trying to avoid.

Perhaps the call to i.flushRateLimiter.Wait should be done after dequeing

This cures the above scenario. I initially found it odd to think we would pull a series from the queue then wait with it, but I don't think this does any harm - we always re-examine the series before sending to the DB, so won't double-flush it.

and only if op.immediate is false?

Excellent idea.

[pstibrany]

LGTM. Left some non-blocking comments.

[pstibrany]

I don't quite understand why this was needed.

[bboreham]

Because CI was failing; not all chunks had been flushed by the time it checked. I could understand that this PR makes the flush take longer, though the queue should clear within one flush period. On my laptop the failure was intermittent; after several hours of probing I found that Go timers just aren't that reliable - I would see the 20ms timer fire much later, sometimes by 100ms (and if that happens the test still fails after this change). Perhaps related to golang/go#38860 though I can't see what would make any goroutines CPU-bound at the point we do this Sleep().

I could take that change back out and see how we get on.

[pstibrany]

I could take that change back out and see how we get on.

No need for me. Thanks for explanation.

[bboreham]

I have rebased to get rid of the merge conflict on CHANGELOG, fixed the 'DB' comment, and dropped the change to wait longer in the test. Let's see if it passes...