HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

Watchdog in Golang

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
golangwatchdogstackoverflow

Problem

I am new to Go. I am trying to implement a simple watchdog. It calls a function once after a certain delay has elapsed; kicking it resets the delay. Kicking it after the function has already been called restarts it.

type watchdog struct {
    interval time.Duration
    timer *time.Timer
}

func NewWatchdog(interval time.Duration, callback func()) *watchdog {
    w := watchdog{
        interval: interval,
        timer: time.AfterFunc(interval, callback),
    }
    return &w
}

func (w *watchdog) Stop() {
    w.timer.Stop()
}

func (w *watchdog) Kick() {
    w.timer.Reset(w.interval)
}


I read https://golang.org/pkg/time/ and understand there might be some caveats around func (*Timer) Reset. My code is totally oblivious of this fact and seems to work nonetheless. I tested a few corner cases: calling Stop() twice in a row, calling Kick() after Stop() or after the function has been called.

Could anyone please let me know if anything is wrong with the code? Please provide test cases (plain English is okay) to crash it or get an unexpected behaviour. Can you spot any inefficiencies?

Solution

Go has a concurrency model that's different to most other languages. You'll get used to it, but the instructions for the usage of the timer.Reset() method are very specific, and are a consequence of Go's concurrency model.

You should follow those instructions.


To reuse an active timer, always call its Stop method first and—if it had expired—drain the value from its channel.

As you have discovered, in most cases, the consequence of not following the instructions are not significant. Where it will become significant is when the timer event happens at (about) the same time as the reset call. You can't create a test case that forces that timing, but at some point, in the future, the timing will happen to coincide, and your program's behaviour will be odd (the timer will tick even though you're calling the reset event, for example).

In your use-case, this concurrency problem may still not be significant. It will create "false positives" - watchdog timeouts for apparently reset watchdogs - but the reset was so close to the watchdog timeout that the watchdog cycle was probably worth triggering anyway.

As for your actual code, it's pretty good. There are some small things to consider.

Firstly, why don't you make it a separate package? At the moment, your watchdog is not exported, and can only be run by things in your package. You export the Stop and Kick methods, but not the actual struct. Create a watchdog folder, and then make a standalone package:

package watchdog;

import (
    "time"
)

type Watchdog struct {
    interval time.Duration
    timer *time.Timer
}

func New(interval time.Duration, callback func()) *Watchdog {
    w := Watchdog{
        interval: interval,
        timer: time.AfterFunc(interval, callback),
    }
    return &w
}

func (w *watchdog) Stop() {
    w.timer.Stop()
}

func (w *watchdog) Kick() {
    w.timer.Stop()
    w.timer.Reset(w.interval)
}


Note that the creation method is renamed to just New. Thus calling code can just:

import "watchdog"
wdog := watchdog.New(time.Second, func() {....})

....

    wdog.Kick()


Finally, I understand why you have the callback function argument/mechanism for your watchdog. There's nothing wrong with that, but there will/may be times when a user of the watchdog would prefer a channel output instead of a callback. Having a channel allows people to merge the watchdog timeout logic with other active logic. For example, a loop like:

wdog := watchdog.New(time.Second)
for {
    wdog.Kick()
    select {
    case <- wdog:
        // watchdog expired. Reset entire system
    case <-input:
        // got data so process it.
    }
}

Code Snippets

package watchdog;

import (
    "time"
)

type Watchdog struct {
    interval time.Duration
    timer *time.Timer
}

func New(interval time.Duration, callback func()) *Watchdog {
    w := Watchdog{
        interval: interval,
        timer: time.AfterFunc(interval, callback),
    }
    return &w
}

func (w *watchdog) Stop() {
    w.timer.Stop()
}

func (w *watchdog) Kick() {
    w.timer.Stop()
    w.timer.Reset(w.interval)
}
import "watchdog"
wdog := watchdog.New(time.Second, func() {....})

....

    wdog.Kick()
wdog := watchdog.New(time.Second)
for {
    wdog.Kick()
    select {
    case <- wdog:
        // watchdog expired. Reset entire system
    case <-input:
        // got data so process it.
    }
}

Context

StackExchange Code Review Q#144273, answer score: 3

Revisions (0)

No revisions yet.