Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crash when allow pending messgae wasn't set #1785

Merged
merged 1 commit into from
Sep 23, 2016
Merged

Fix crash when allow pending messgae wasn't set #1785

merged 1 commit into from
Sep 23, 2016

Conversation

v9n
Copy link
Contributor

@v9n v9n commented Sep 19, 2016

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

I got this error when forgetting to set allow_pending_message on 1.0.0 version.

2016/09/19 00:14:36 ERROR: statsd message queue full. We have dropped 1 messages so far. You may want to increase allowed_pending_messages in the config
panic: runtime error: integer divide by zero
[signal 0x8 code=0x1 addr=0x621709 pc=0x621709]

goroutine 21 [running]:
panic(0x12cefe0, 0xc82000a060)
    /usr/local/go/src/runtime/panic.go:481 +0x3e6
github.com/influxdata/telegraf/plugins/inputs/statsd.(*Statsd).udpListen(0xc8200cc0d0, 0x0, 0x0)
    /home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/statsd/statsd.go:298 +0x7d9
created by github.com/influxdata/telegraf/plugins/inputs/statsd.(*Statsd).Start
    /home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/statsd/statsd.go:261 +0x2a7

The default is 0 so we hit a division by 0 error and crash. This checks
ensure we will not crash and log and continue to let telegraf run

@sparrc
Copy link
Contributor

sparrc commented Sep 19, 2016

can you also set the default value for AllowedPendingMessages in the init() function?

@v9n
Copy link
Contributor Author

v9n commented Sep 21, 2016

@sparrc Updated.

sparrc
sparrc suggested changes Sep 21, 2016
View reviewed changes
plugins/inputs/statsd/statsd.go
@@ -24,7 +24,8 @@ const (

defaultFieldName = "value"

defaultSeparator = "_"
defaultSeparator = "_"
defaultAllowPendingMessage = 100000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be 10000, not 100000

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@nhaugo nhaugo added this to the 1.1.0 milestone Sep 21, 2016
@jwilder jwilder added bug unexpected problem or unintended behavior panic issue that results in panics from Telegraf labels Sep 21, 2016
Fix crash when allow pending messgae wasn't set
fe30959 
The default is 0 so we hit a division by 0 error and crash. This checks
ensure we will not crash and `log` and continue to let telegraf run

Also we set default allow pending message number to 10000
@v9n
Copy link
Contributor Author

v9n commented Sep 22, 2016

@sparrc I correct the defautl value :(. Sorry about that.

@sparrc
Copy link
Contributor

sparrc commented Sep 23, 2016

thanks @kureikain!

@sparrc sparrc merged commit 1d10eda into influxdata:master Sep 23, 2016
sparrc pushed a commit that referenced this pull request Sep 23, 2016
@sparrc
Fix crash when allow pending messgae wasn't set (#1785)
beba50c 
The default is 0 so we hit a division by 0 error and crash. This checks
ensure we will not crash and `log` and continue to let telegraf run

Also we set default allow pending message number to 10000
@sparrc sparrc modified the milestones: 1.0.1, 1.1.0 Sep 23, 2016
@sparrc sparrc mentioned this pull request Oct 4, 2016
Closed
jackzampolin pushed a commit that referenced this pull request Oct 7, 2016
@jackzampolin
Fix crash when allow pending messgae wasn't set (#1785)
3d91f22 
The default is 0 so we hit a division by 0 error and crash. This checks
ensure we will not crash and `log` and continue to let telegraf run

Also we set default allow pending message number to 10000
@sparrc sparrc mentioned this pull request Oct 10, 2016
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sparrc sparrc sparrc approved these changes

Assignees
No one assigned
Labels
bug unexpected problem or unintended behavior panic issue that results in panics from Telegraf
Projects
None yet
Milestone
1.0.1
Development

Successfully merging this pull request may close these issues.
4 participants
@v9n @sparrc @jwilder @nhaugo