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

processes: increase reading buffer for /proc/stat #3479

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

mhumpula
Copy link
Contributor

@mhumpula mhumpula commented Jun 11, 2020

If machine has high number of CPUs or interrupts or both, the /proc/stat
can easily grow over current buffer size 4kB. On my current machine (64
cores, ~300 interrupts) the /proc/stat has 12kB of data. And since the
procs_running line is after the interrupts it is never read.

Increasing the size to 64KB so it will accommodate even heavier machines.

It would be nice if this would be more dynamic to don't allocate large buffer on smaller machines. I have no idea about the current smallest supported setup by the collectd. Optionally the part could be reprogrammed to read the whole file in 4kB chunks, something similar is already done in read_fork_rate.

ChangeLog: Process plugin: Increase reading buffer for /proc/stat

@mhumpula mhumpula requested a review from a team as a code owner June 11, 2020 14:48
@mrunge
Copy link
Member

mrunge commented Jun 19, 2020

Thank you for your contribution. This looks okay to me. Would you be able to run

contrib/format.sh src/processes.c 

and push that change? Apparently, this is required for CI to pass here.

If machine has high number of CPUs or interrupts or both, the /proc/stat
can easily grow over current buffer size 4kB. On my current machine (64
cores, ~300 interrupts) the /proc/stat has 12kB of data. And since the
`procs_running` line is after the interrupts it is never read.

Increasing the size to 64KB so it will accommodate even heavier machines.
@mhumpula
Copy link
Contributor Author

I didn't wont to change non related lines, but ok. It's pushed reformated now.

Copy link
Member

@sunkuranganath sunkuranganath left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
This change looks good for now.
Ideally this plugin could be updated overall to be more efficient.

@sunkuranganath sunkuranganath merged commit f434908 into collectd:master Jun 19, 2020
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.

3 participants