WaitStage for Grafana Alertmanager #278
Conversation
yuri-tceretian
force-pushed
the
yuri-tceretian/wait-stage
branch
from
a704c1a to
20a5381
Compare
| t := time.NewTimer(wait) | ||
| defer t.Stop() |
There was a problem hiding this comment.
Didn't know about this, good catch. It seems like this was fixed in 1.23 though. As per the time.After docs:
Before Go 1.23, this documentation warned that the underlying Timer would not be recovered by the garbage collector until the timer fired, and that if efficiency was a concern, code should use NewTimer instead and call Timer.Stop if the timer is no longer needed. As of Go 1.23, the garbage collector can recover unreferenced, unstopped timers. There is no reason to prefer NewTimer when After will do.
Considering Grafana is already in 1.23, we could consider updating alerting and get this for free. We could keep this for the logs in any case - wdyt?
There was a problem hiding this comment.
Good catch :) can change it back then
This PR introduces a new implementation of WaitStage that has two differences from the default one:
time.Afterthat leaks if the context is stopped (for example when the configuration is updated), it uses time.Timer with proper finalization