espadolini
commented
2 days ago I renamed the type to Delay and the channel method to Elapsed rather than Next, but I left Advance because it fits and because ResetFrom might give the idea that it's callable in more situations than it should.
I think it'll be too easy for folks to assume it works like a time.Ticker or the existing Interval helper, and not be sufficiently diligent about resetting.
TBH I'm not entirely sure how you can misuse a Delay without it immediately deadlocking (or very obviously overticking, in the best case scenario), but the subtlety needed in its use is the reason why it's an internal package in lib/inventory rather than just being in utils or something like that - the inconvenience of hoisting the logic up in the user's select loop is justified if there's a tangible benefit in having fewer running goroutines, which is not true often.
public-teleport-github-review-bot[bot]
This PR replaces the
MultiIntervalused by the inventory controller with a lightweight wrapper around atime.Timerthat doesn't make use of goroutines, gets rid of "control log" inventory pings (as the control log mechanism is not sound given the changes to instance heartbeats, and since nothing uses it, there's no reason to exercise it) so we no longer need a method to trigger instance heartbeats from the outside of the inventory, removes an unstable envvar that disables heartbeat keepalives (the largest setup we know of didn't end up needing it) and prepares the inventory controller to make use of variable rate heartbeats, which are going to be per-protocol in a later PR (that will likely not get backported, unlike this one).