Skip to content

Commit

Permalink
Do not create a global statsd "previous instance"
Browse files Browse the repository at this point in the history
this basically reverts influxdata#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 influxdata#1975
fixes influxdata#2102
  • Loading branch information
sparrc authored and Nick White committed Jan 31, 2017
1 parent 75b0e31 commit 505b183
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ plugins, not just statsd.
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
Expand Down Expand Up @@ -91,6 +100,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]

Expand Down
18 changes: 4 additions & 14 deletions plugins/inputs/statsd/statsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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," +
Expand All @@ -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
}

Expand Down

0 comments on commit 505b183

Please sign in to comment.