added a periodic worker

[mattyw]

The periodic worker will call a function periodically based on an input duration

[fwereade]

if err != nil we should kill the tomb and return immediately

[fwereade]

Thanks, but needs a few tweaks.

[rogpeppe]

time.Tick is wrong here - it creates a new Ticker which is never destroyed,
which could become a serious memory leak

You want to use time.Timer. Something like this, perhaps?

t := time.NewTimer(0)
w := &periodicWorker{}
go func() {
    defer w.tomb.Done()
    for {
        if err := doWork(w.tomb.Dying());  err != nil {
             w.tomb.Kill(err)
             return
        }
        t.Reset(sleepDuration)
        select {
        case <-t.C:
        case <-w.tomb.Dying()
            return
        }
   }

[mattyw]

Great catch @rogpeppe, thanks very much

[mattyw]

@rogpeppe @fwereade thanks for the feedback. PTAL

[fwereade]

This feels a little bit off. I'm thinking something more like:

type Callback func(stop <-chan struct{}) error

func NewPeriodicWorker(callback Callback, period time.Duration) Worker {
    w := periodicWorker{}
    go func() {
        defer w.tomb.Done()
        w.tomb.Kill(w.run(callback, period))
    }
    return w
}

func (w *periodicWorker) run(callback Callback, period time.Duration) error {
    tick := time.After(0)
    stop := w.tomb.Dying()
    for {
        select {
        case <-stop:
            return tomb.ErrDying
        case <-tick:
            if err := callback(stop); err != nil {
                return err
            }
            tick = time.After(period)
        }
    }
}

[fwereade]

your conscientiousness does you credit, I often get lazy and just skip error checking in those cases. I should stop doing that.

[fwereade]

One significant concern re tomb behaviour, otherwise looking great.

[fwereade]

This is just what I was looking for, but would you parameterise it on the test error and run it twice please? Just so we can verify the trivial effect of a nil return from a stopped work func.

[fwereade]

LGTM with one trivial variation on a test.

[mattyw]

$$merge$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju