From 73acd114d152bc03f81ad8c07c5043becb567dd0 Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Tue, 20 Dec 2016 17:48:06 +0000 Subject: [PATCH] Do not create a global statsd "previous instance" this basically reverts #887 at some point we might want to do some special handling of reloading plugins and keeping their state intact, but that will need to be done at a higher level, and in a way that is thread-safe for multiple input plugins of the same type. Unfortunately this is a rather large feature that will not have a quick fix available for it. fixes #1975 fixes #2102 --- CHANGELOG.md | 10 ++++++++++ plugins/inputs/statsd/statsd.go | 18 ++++-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b85e95195..ed25cf7f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ will change te default behavior for users who were not specifying these parameters in their config file. +- The StatsD plugin will also no longer save it's state on a service reload. +Essentially we have reverted PR [#887](https://github.com/influxdata/telegraf/pull/887). +The reason for this is that saving the state in a global variable is not +thread-safe (see [#1975](https://github.com/influxdata/telegraf/issues/1975) & [#2102](https://github.com/influxdata/telegraf/issues/2102)), +and this creates issues if users want to define multiple instances +of the statsd plugin. Saving state on reload may be considered in the future, +but this would need to be implemented at a higher level and applied to all +plugins, not just statsd. + ### Features - [#2123](https://github.com/influxdata/telegraf/pull/2123): Fix improper calculation of CPU percentages @@ -52,6 +61,7 @@ in their config file. - [#1449](https://github.com/influxdata/telegraf/issues/1449): MongoDB plugin always shows 0 replication lag. - [#1825](https://github.com/influxdata/telegraf/issues/1825): Consul plugin: add check_id as a tag in metrics to avoid overwrites. - [#1973](https://github.com/influxdata/telegraf/issues/1973): Partial fix: logparser CLF pattern with IPv6 addresses. +- [#1975](https://github.com/influxdata/telegraf/issues/1975) & [#2102](https://github.com/influxdata/telegraf/issues/2102): Fix thread-safety when using multiple instances of the statsd input plugin. ## v1.1.2 [2016-12-12] diff --git a/plugins/inputs/statsd/statsd.go b/plugins/inputs/statsd/statsd.go index 1b0189a9e..d2c627b8a 100644 --- a/plugins/inputs/statsd/statsd.go +++ b/plugins/inputs/statsd/statsd.go @@ -32,8 +32,6 @@ var dropwarn = "E! Error: statsd message queue full. " + "We have dropped %d messages so far. " + "You may want to increase allowed_pending_messages in the config\n" -var prevInstance *Statsd - type Statsd struct { // Address & Port to serve from ServiceAddress string @@ -244,17 +242,10 @@ func (s *Statsd) Start(_ telegraf.Accumulator) error { s.done = make(chan struct{}) s.in = make(chan []byte, s.AllowedPendingMessages) - if prevInstance == nil { - s.gauges = make(map[string]cachedgauge) - s.counters = make(map[string]cachedcounter) - s.sets = make(map[string]cachedset) - s.timings = make(map[string]cachedtimings) - } else { - s.gauges = prevInstance.gauges - s.counters = prevInstance.counters - s.sets = prevInstance.sets - s.timings = prevInstance.timings - } + s.gauges = make(map[string]cachedgauge) + s.counters = make(map[string]cachedcounter) + s.sets = make(map[string]cachedset) + s.timings = make(map[string]cachedtimings) if s.ConvertNames { log.Printf("I! WARNING statsd: convert_names config option is deprecated," + @@ -271,7 +262,6 @@ func (s *Statsd) Start(_ telegraf.Accumulator) error { // Start the line parser go s.parser() log.Printf("I! Started the statsd service on %s\n", s.ServiceAddress) - prevInstance = s return nil }