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.

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

Default SNMP parameter changes #1813

Merged
merged 1 commit into from
Sep 28, 2016
Merged

Default SNMP parameter changes #1813

merged 1 commit into from
Sep 28, 2016

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Sep 26, 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)

max-repetitions = 10 is the default of net-snmp utils according to
http://net-snmp.sourceforge.net/docs/man/snmpbulkwalk.html

retries = 3 is the default of gosnmp:
https://godoc.org/github.com/soniah/gosnmp#pkg-variables

Could deal with some parts of the performance issues reported
by #1665

@sparrc
Copy link
Contributor Author

sparrc commented Sep 26, 2016

cc @phemmer can you take a look? what do you think of changing these?

@@ -105,7 +109,7 @@ type Snmp struct {
Community string

// Parameters for Version 2 & 3
MaxRepetitions uint
MaxRepetitions int
Copy link
Contributor

@phemmer phemmer Sep 26, 2016

Choose a reason for hiding this comment

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

Just FYI, if this was changed to avoid the conversion on line 613, it'll change again when the gosnmp lib is updated, as it's now a uint8.

@phemmer
Copy link
Contributor

phemmer commented Sep 26, 2016

Looks good to me

@sparrc sparrc force-pushed the cs1665 branch 2 times, most recently from cacd84c to d1f68b6 Compare September 28, 2016 13:32
max-repetitions = 10 is the default of net-snmp utils according to
http://net-snmp.sourceforge.net/docs/man/snmpbulkwalk.html

retries = 3 is the default of gosnmp:
https://godoc.org/github.com/soniah/gosnmp#pkg-variables

Could deal with some parts of the performance issues reported
by #1665
@sparrc sparrc merged commit e7e39df into master Sep 28, 2016
@sparrc sparrc deleted the cs1665 branch September 28, 2016 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants