reduce timer object creation to reduce gc overhead

[es-chow]

by start 3 nodes with GODEBUG=memprofilerate=1, and using

http://host:port/debug/pprof/heap?debug=1

will found heavy timer object creation

1604: 102656 [4251: 272064] @ 0x9cb15c 0x9cb451 0xa0b681 0x94dcd1
#   0x9cb15c    time.NewTimer+0x6c                              /usr/local/go/src/time/sleep.go:74
#   0x9cb451    time.After+0x21                                 /usr/local/go/src/time/sleep.go:110
#   0xa0b681    github.com/cockroachdb/cockroach/server.(*rpcTransport).processQueue+0x781  /cockroach/src/github.com/cockroachdb/cockroach/server/raft_transport.go:166

This creation can be done only once.

[petermattis]

I'm not sure if this is functionally equivalent. The old code would restart the timer on every iteration. But it looks like this new pattern introduces a small race where the timer can fire just as a request is received. The timer internally sends to raftIdleTimer.C and on the next iteration we call Reset, but that doesn't clear the channel. I'm not familiar enough with this code to know if that race is innocuous.

[bdarnell]

Good catch; that does seem like an issue. Now that I look closely, there are several other problems with the shutdown-on-idle behavior. A new request could be written to the channel after the decision has been made to shut down the goroutine, and even without any races the goroutine could stop while there are still outstanding requests that will try to write to the done channel.

We have several instances of this pattern where we want a goroutine that goes away when it is idle (here, gossip.infoStore.runCallbacks, and my upcoming work on parallelizing raft execution). I'm going to try to refactor the infoStore pattern into something reusable.

[tamird]

Reset returns false if the timer had expired (in other words, it returns false if there's a value parked on the channel). Seems easy enough to mitigate:

if !raftIdleTimer.Reset(raftIdleTimeout) {
  <-raftIdleTimer.C
}

[bdarnell]

LGTM. While there are some possible races, I don't think this change makes things significantly worse than before so we might as well make the change.

[es-chow]

close as #4367 will fix the same.