server: fix timer leak #900
Conversation
|
@bmizerany finds out that there's tons of Timer that that are not recycled, which is because the timer vars used in |
|
I'm still waiting on the profiler to finish. This could be awhile. |
|
Okay. After 2e6 inserts with this patch, all timers previously left on the heap are now gone. Great job! LGTM. |
|
NOTE: There is memory growth as the indexes get larger. This is believed to be due to the index being stored as a string in the log entries. This bloat is only noticable in the early life of etcd and smooths out pretty quick since it takes longer to crank it up an extra digit. |
| @@ -772,9 +772,9 @@ func (s *PeerServer) startRoutine(f func()) { | |||
| func (s *PeerServer) monitorSnapshot() { | |||
| for { | |||
| timer := time.NewTimer(s.snapConf.checkingInterval) | |||
There was a problem hiding this comment.
Sorry to jump in, but wondering why a timer here instead of a ticker outside of the loop?
There was a problem hiding this comment.
Good catch. @unihorn, can you take a look?
There was a problem hiding this comment.
@aybabtme I thought snapshot may take rather long time compared to checkingInterval, and this could cache many ticks. But actually ticker drops these if there is an appending one.
I think it should be better to use ticker now.
@xiangli-cmu @bmizerany