nats.go
nats.go copied to clipboard
Remove globalTimerPool global variable
We occasionally see weird behavior when using Request with timeouts where it appeared as though the timer had already fired even though it did not appear to be the case. Looking through the Request functions, it seemed pragmatic to rely on the standard library's context
instead. The other use of globalTimerPool
in NextMsg
and FlushTimeout
appeared to also be able to use context.WithTimeout
which made it no longer necessary to have the globalTimerPool
variable.
Looking through some open issues, it appeared as though https://github.com/nats-io/nats.go/issues/880 could be addressed with this change.
Coverage decreased (-0.09%) to 85.159% when pulling 4533c937c395c23643dd1623451cabb414f1a7ca on jipperinbham:jp/no-global-pool into c4cfcdf430719cd3e02ded4b5412c0925fcbfb64 on nats-io:main.
Thanks for the contribution. I think this might be preventing the issue because context
is not using a pool for those timers and avoiding what looks like were timer
regressions in go 1.16, 1.7 that have been fixed since then? (https://github.com/golang/go/issues/47859)
Internally context.WithTimeout
uses AfterFunc
so there would be a goroutine per request, not sure about changing that into being the default yet...
Also have you tried with switching to RequestWithContext
instead?