Gather concurrently from snmp agents #3365
Conversation
plugins/inputs/snmp/snmp.go
Outdated
| if s.initialized { | ||
| return nil | ||
| } | ||
|
|
||
| s.connectionCache = make(map[int]snmpConnection) |
There was a problem hiding this comment.
Since we're changing the cache key to be the agent's index in the list, we can turn this into a slice instead, using the index in s.Agents as the index for s.connectionCache. And then since we'd be using a slice, we can pre-allocate it here as we know the size of s.Agents. And then once we pre-allocate it, we can do away with the mutex since it'll never change during runtime. The only change will be to individual elements, which the updated description for getConnection says is an error to access in more than 1 goroutine.
None of that is required, just throwing it out there :-)
There was a problem hiding this comment.
Good point, I'll do this.
plugins/inputs/snmp/snmp.go
Outdated
| // result using `agentIndex` as the cache key. This is done to allow multiple | ||
| // connections to a single address. It is an error to use a connection in | ||
| // more than one goroutine. | ||
| func (s *Snmp) getConnection(agentIndex int, agent string) (snmpConnection, error) { |
There was a problem hiding this comment.
agent param is redundant. You can get it from s.Agents[agentIndex].
|
@phemmer I updated the pull request with your suggestions, can you take a second look? |
|
Looks good to me |
Adds concurrent collection for each agent, each agent is still gathered sequentially.
I changed the connection cache to hold connections by index, in case the agent list contains multiples of the same destination.
#1665
Required for all PRs: