Skip to content

Commit

Permalink
Review feedback changes 1
Browse files Browse the repository at this point in the history
  • Loading branch information
flotter committed Aug 16, 2024
1 parent 7103bb8 commit 6dcb83f
Showing 1 changed file with 8 additions and 13 deletions.
21 changes: 8 additions & 13 deletions internals/overlord/overlord.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,19 +328,14 @@ func (o *Overlord) ensureTimerSetup() {
o.pruneTicker = time.NewTicker(pruneInterval)
}

func (o *Overlord) ensureTimerStopClearChannel() {
func (o *Overlord) ensureTimerStop() {
o.ensureLock.Lock()
defer o.ensureLock.Unlock()
// The Go v1.x timer implementation does not guarantee that a Reset
// clears the channel. The documented proposal is to call stop and
// manually clear the channel if it already expired. This is required
// on loop entry because the timer is started much earlier, and
// managers are now allowed to call state.EnsureBefore() as soon as state
// is available. If the ensure timer expired before loop entry, this code
// ensures the channel is cleared and that only the ensure on entry is
// performed (and not a second ensure as a result of the channel).
// See: https://pkg.go.dev/time#Timer.Reset
o.ensureTimer.Stop()
// The Go version < 1.23 timer implementation does not guarantee that a
// Stop clears the channel. The documented proposal is to call stop and
// manually clear the channel if it already expired.
// See: https://pkg.go.dev/time#Timer.Stop
select {
case <-o.ensureTimer.C:
default:
Expand Down Expand Up @@ -384,10 +379,10 @@ func (o *Overlord) ensureBefore(d time.Duration) {
// Loop runs a loop in a goroutine to ensure the current state regularly through StateEngine Ensure.
func (o *Overlord) Loop() {
o.loopTomb.Go(func() error {
// We are about to perform the first ensure on loop entry. Stop and
// clear the timer channel, so we do not process a spurious second
// We are about to perform the first ensure on loop entry. Stop (and
// clear the timer channel), so we do not process a spurious second
// ensure due to a pending timer channel expiry.
o.ensureTimerStopClearChannel()
o.ensureTimerStop()
for {
// TODO: pass a proper context into Ensure
o.ensureTimerReset()
Expand Down

0 comments on commit 6dcb83f

Please sign in to comment.