Use channel to stop the node.Refresh() goroutine

[oliver006]

Unfortunately, refreshTicker.Stop() doesn't close the refreshTicker channel so the goroutines refreshing the node status don't ever exit and goroutines start accumulating every time you open a new session to a cluster.
This PR changes the code to use a bool chan instead of the ticker channel.

[dancannon]

Thanks for spotting this issue, leaking goroutines can be as easy as creating them! What do you think of just doing something like this instead?

n.refreshTicker.Stop()
close(n.refreshTicker.C)

[oliver006]

Yeah, that was my first thought as well but turns out, you can't close the ticker channel, it's read-only.
See http://stackoverflow.com/questions/17797754/ticker-stop-behaviour-in-golang for details.

[dancannon]

You could use a chan struct{} here instead. Not much difference but theres no reason not to.

[oliver006]

Kinda the same but I slightly prefer the bool chan as it somewhat better communicates the Done == true fact. No strong feelings though.

[dancannon]

Oh interesting, I will give it a proper test tomorrow then. Thanks.

[oliver006]

Sounds good.

[oliver006]

fyi, ran this with the fixed version for a while, you can spot the difference quite easily:

[kofalt]

@oliver006 Pardon the intrusion - what user interface is that screenshot from?

[oliver006]

@kofalt - no worries. The data is collected via Prometheus and the UI is PromDash
I wrote a rethinkdb metrics exporter for Prometheus which you can find here if you're interested: https://github.com/oliver006/rethinkdb_exporter