backoff: fix flaky tests in backoff cache #1516
Conversation
| delete(c.sendingChs, ch) | ||
| break | ||
| } else if rem > 0 { | ||
| rem-- |
There was a problem hiding this comment.
This is broken, right? This is a copy of an int not a reference so it wouldn't affect the rem value in the map at all.
| ch <- sendAi | ||
| if rem == 1 { | ||
| close(ch) | ||
| delete(c.sendingChs, ch) |
There was a problem hiding this comment.
There's no point doing this here, we can clean everything up in one go at the end.
There was a problem hiding this comment.
In other parts of the code, we use https://github.com/benbjohnson/clock to mock the clock. Can we reuse that package here?
Sure thing. Taking a look at that library I noticed they used a pointer receiver though, which goes against my goal of having the real clock be zero sized. It doesn't actually matter I guess but worth pointing out. Here's some benchmarks: The pointer receiver version is 45ns/op vs 38ns/op. |
|
I can actually keep the real clock as I have it here and use this only in tests. |
marten-seemann
changed the title
Fixes #1502 by introducing a mocked clock. Time can be incremented in the mock clock and things check the clocks time via
.Now(). If no clock is passed in, the default realClock usestime.NowThis means that these tests are not dependent on actual wall clock times, but our monotonically increasing mock clock.
This PR also fixes a subtle bug that would cause the limit of a
sendingChsto be ignored. And a bug that prevented othersendingChsfrom getting sent data.