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

Add setting to enable caching in ipmitool #8335

Merged
merged 7 commits into from
Jan 21, 2021

Conversation

DEvil0000
Copy link
Contributor

@DEvil0000 DEvil0000 commented Oct 29, 2020

I had the issue that ipmi data was reported with gaps on a 30s interval on HP G10 servers (latest firmware).
The gaps where due to the ipmitool command was not always finishing within the 20s timeout. Even changeing the timeout to 30s was not always giving it enough time to respond.
ipmi-sensors from freeipmi tools which is used in some prometheus ipmi exporters is running way faster but does not provide all information I needed.
At some point I just realized there is a cache option for ipmitool. Using this option speeds the tool up by something between 3x and 6x for most of my runs so it holds the deadline given by timeout and I can record the data not even in 30s intervals but also now in 15s intervals.

In case you merge this or agree with it, I would appreciate a hacktoberfest-accepted label.

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
@DEvil0000
Copy link
Contributor Author

@reimda I am under the impression your requested changes marked the checks failed. Can you tell me whats wrong about it now?

@p-zak
Copy link
Collaborator

p-zak commented Nov 2, 2020

@reimda I am under the impression your requested changes marked the checks failed. Can you tell me whats wrong about it now?

cd plugins/inputs/ipmi_sensor
go fmt ipmi.go

Moreover, please rebase on master.

@DEvil0000 DEvil0000 force-pushed the improved_ipmitool_performance branch 2 times, most recently from 772154c to d2015d3 Compare November 2, 2020 13:34
@DEvil0000
Copy link
Contributor Author

rebased and formatted as requested.

Please add hacktoberfest-accepted label on merge

@DEvil0000 DEvil0000 requested a review from reimda November 2, 2020 14:01
@DEvil0000
Copy link
Contributor Author

@reimda do you still see something?

@sjwang90 sjwang90 added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Nov 24, 2020
@sjwang90 sjwang90 added this to the Planned milestone Dec 9, 2020
@zak-pawel
Copy link
Collaborator

@DEvil0000 PR still needs to be rebased.

@DEvil0000 DEvil0000 force-pushed the improved_ipmitool_performance branch from dc0585f to 43c5fd2 Compare January 20, 2021 16:50
@DEvil0000
Copy link
Contributor Author

@zak-pawel rebase done.

@DEvil0000 DEvil0000 requested a review from p-zak January 20, 2021 18:33
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Outdated Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Show resolved Hide resolved
plugins/inputs/ipmi_sensor/ipmi.go Show resolved Hide resolved
@zak-pawel zak-pawel 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 Jan 21, 2021
@reimda reimda changed the title improved ipmitool performance by using cache file Add setting to enable caching in ipmitool Jan 21, 2021
@reimda reimda merged commit 358633b into influxdata:master Jan 21, 2021
ssoroka pushed a commit that referenced this pull request Jan 27, 2021
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants