feat: Optimize locking for SNMP MIBs loading. #10206

srebhan · 2021-12-02T14:28:45Z

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

This PR optimizes locking for the SNMP MIBs loading. This is necessary as the current loading of those files is completely serial. With large setups with dozends or hundreds of SNMP plugins this might take long...

@Hipska will provide measurements... :-)

Hipska · 2021-12-02T15:31:54Z

My setup is using around 1700 snmp input instances.

Current release (1.20.4)

2021-12-02T14:12:40Z I! Starting Telegraf 1.20.4
2021-12-02T14:12:41Z D! [agent] Initializing plugins
2021-12-02T14:12:41Z D! [agent] Connecting outputs
...

Note: This version still used snmptranslate and snmptable on the fly when the input is started. (this takes around 10s on a fast system)

Current master (1.21.0 RC0)

2021-12-02T14:19:43Z I! Starting Telegraf 1.21.0
2021-12-02T14:19:44Z D! [agent] Initializing plugins

After this, I waited for 30 minutes until I killed it. CPU was high and memory usage grew steadily to 14GiB during that time.

Artefact `0e707eb`

2021-12-02T15:05:14Z I! Starting Telegraf 1.21.0-0e707ebc
2021-12-02T15:05:16Z D! [agent] Initializing plugins

Similar behaviour as RC0, stopped after 23 minutes and 10GiB of memory usage.

srebhan · 2021-12-02T17:07:12Z

With the added benchmark, i.e. loading the testdata directory multiple times I see the following

Current master

goos: linux
goarch: amd64
pkg: github.com/influxdata/telegraf/plugins/inputs/snmp
cpu: AMD Ryzen 7 3700X 8-Core Processor             
BenchmarkMibLoading
BenchmarkMibLoading-16                 3         438131129 ns/op
PASS

Optimized locking

goos: linux
goarch: amd64
pkg: github.com/influxdata/telegraf/plugins/inputs/snmp
cpu: AMD Ryzen 7 3700X 8-Core Processor             
BenchmarkMibLoading
BenchmarkMibLoading-16                 3         452756498 ns/op
PASS

Caching

goos: linux
goarch: amd64
pkg: github.com/influxdata/telegraf/plugins/inputs/snmp
cpu: AMD Ryzen 7 3700X 8-Core Processor             
BenchmarkMibLoading
BenchmarkMibLoading-16           4237710               288.9 ns/op
PASS

reimda · 2021-12-02T21:17:53Z

@Hipska Do you see acceptable performance with 336b0ce?

Hipska · 2021-12-03T08:37:42Z

Artefact `336b0ce`

2021-12-03T08:35:15Z I! Starting Telegraf 1.21.0-336b0ced
2021-12-03T08:35:16Z D! [agent] Initializing plugins
2021-12-03T08:35:21Z D! [agent] Connecting outputs
...

Whoa, this is what it should have been!

Hipska

Would you willing to change the log messages a little bit while we are here?

internal/snmp/translate.go

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@me.com>

telegraf-tiger · 2021-12-03T10:27:00Z

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here ! 🐯

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_armv7.tar.gz
armhf.deb	i386.rpm	freebsd_i386.tar.gz
i386.deb	ppc64le.rpm	linux_amd64.tar.gz
mips.deb	s390x.rpm	linux_arm64.tar.gz
mipsel.deb	x86_64.rpm	linux_armel.tar.gz
ppc64el.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

MyaLongmire

Thanks for doing this Sven

srebhan · 2021-12-03T20:15:23Z

@MyaLongmire thanks for making this possible in the first place. That PR was fun. :-)

(cherry picked from commit 2d420fb)

…)" This reverts commit 2d420fb.

* origin/master: (133 commits) chore: restart service if it is already running and upgraded via RPM (influxdata#9970) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10237) fix: Handle duplicate registration of protocol-buffer files gracefully. (influxdata#10188) fix(http_listener_v2): fix panic on close (influxdata#10132) feat: add Vault input plugin (influxdata#10198) feat: support aws managed service for prometheus (influxdata#10202) fix: Make telegraf compile on Windows with golang 1.16.2 (influxdata#10246) Update changelog feat: Modbus add per-request tags (influxdata#10231) fix: Implement NaN and inf handling for elasticsearch output (influxdata#10196) feat: add nomad input plugin (influxdata#10106) fix: Print loaded plugins and deprecations for once and test (influxdata#10205) fix: eliminate MIB dependency for ifname processor (influxdata#10214) feat: Optimize locking for SNMP MIBs loading. (influxdata#10206) feat: Add SMART plugin concurrency configuration option, nvme-cli v1.14+ support and lint fixes. (influxdata#10150) feat: update configs (influxdata#10236) fix(inputs/kube_inventory): set TLS server name config properly (influxdata#9975) fix: Sudden close of Telegraf caused by OPC UA input plugin (influxdata#10230) fix: bump github.com/eclipse/paho.mqtt.golang from 1.3.0 to 1.3.5 (influxdata#9913) fix: json_v2 parser timestamp setting (influxdata#10221) fix: ensure graylog spec fields not prefixed with '_' (influxdata#10209) docs: remove duplicate links in CONTRIBUTING.md (influxdata#10218) fix: pool detection and metrics gathering for ZFS >= 2.1.x (influxdata#10099) fix: parallelism fix for ifname processor (influxdata#10007) chore: Forbids "log" package only for aggregators, inputs, outputs, parsers and processors (influxdata#10191) docs: address documentation gap when running telegraf in k8s (influxdata#10215) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10211) fix: mqtt topic extracting no longer requires all three fields (influxdata#10208) fix: windows service - graceful shutdown of telegraf (influxdata#9616) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10201) feat: Modbus support multiple slaves (gateway feature) (influxdata#9279) fix: Revert unintented corruption of the Makefile from influxdata#10200. (influxdata#10203) chore: remove triggering update-config bot in CI (influxdata#10195) Update changelog feat: Implement deprecation infrastructure (influxdata#10200) fix: extra lock on init for safety (influxdata#10199) fix: resolve influxdata#10027 (influxdata#10112) fix: register bigquery to output plugins influxdata#10177 (influxdata#10178) fix: sysstat use unique temp file vs hard-coded (influxdata#10165) refactor: snmp to use gosmi (influxdata#9518) ...

Optimize locking for SNMP MIBs loading.

0e707eb

telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Dec 2, 2021

Hipska added area/snmp plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Dec 2, 2021

Hipska self-assigned this Dec 2, 2021

Do not load the same directory multiple times.

a1e241d

Sven Rebhan added 2 commits December 2, 2021 17:25

Do proper locking for the cache.

f90749c

Add benchmark for MIB loading.

336b0ce

Hipska approved these changes Dec 3, 2021

View reviewed changes

internal/snmp/translate.go Outdated Show resolved Hide resolved

internal/snmp/translate.go Outdated Show resolved Hide resolved

internal/snmp/translate.go Outdated Show resolved Hide resolved

srebhan and others added 3 commits December 3, 2021 11:06

Avoid duplicate error text.

0511f18

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@me.com>

Improve error message.

8f8732f

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@me.com>

Improve error message.

fc04c01

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@me.com>

Hipska approved these changes Dec 3, 2021

View reviewed changes

Hipska added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Dec 3, 2021

Hipska requested review from reimda and MyaLongmire December 3, 2021 11:31

MyaLongmire approved these changes Dec 3, 2021

View reviewed changes

reimda approved these changes Dec 7, 2021

View reviewed changes

reimda merged commit 2d420fb into influxdata:master Dec 7, 2021

MyaLongmire pushed a commit that referenced this pull request Dec 8, 2021

feat: Optimize locking for SNMP MIBs loading. (#10206)

eff5350

(cherry picked from commit 2d420fb)

nward added a commit to nward/telegraf that referenced this pull request Jan 12, 2022

Revert "feat: Optimize locking for SNMP MIBs loading. (influxdata#10206…

1b4ba85

…)" This reverts commit 2d420fb.

srebhan mentioned this pull request Jan 12, 2022

inputs.snmp walk requests OIDs multiple times and causes high device load #10427

Closed

powersj pushed a commit to powersj/telegraf that referenced this pull request Jan 21, 2022

feat: Optimize locking for SNMP MIBs loading. (influxdata#10206)

5d798de

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Optimize locking for SNMP MIBs loading. #10206

feat: Optimize locking for SNMP MIBs loading. #10206

srebhan commented Dec 2, 2021

Hipska commented Dec 2, 2021 •

edited

Loading

srebhan commented Dec 2, 2021

reimda commented Dec 2, 2021

Hipska commented Dec 3, 2021

Hipska left a comment

telegraf-tiger bot commented Dec 3, 2021

Artifact URLs

MyaLongmire left a comment

srebhan commented Dec 3, 2021

feat: Optimize locking for SNMP MIBs loading. #10206

feat: Optimize locking for SNMP MIBs loading. #10206

Conversation

srebhan commented Dec 2, 2021

Hipska commented Dec 2, 2021 • edited Loading

Current release (1.20.4)

Current master (1.21.0 RC0)

Artefact 0e707eb

srebhan commented Dec 2, 2021

Current master

Optimized locking

Caching

reimda commented Dec 2, 2021

Hipska commented Dec 3, 2021

Artefact 336b0ce

Hipska left a comment

Choose a reason for hiding this comment

telegraf-tiger bot commented Dec 3, 2021

Artifact URLs

MyaLongmire left a comment

Choose a reason for hiding this comment

srebhan commented Dec 3, 2021

Hipska commented Dec 2, 2021 •

edited

Loading

Artefact `0e707eb`

Artefact `336b0ce`