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

Count usage of process start-up #139

Merged
merged 2 commits into from
May 16, 2020

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Mar 26, 2020

When a process started between two Update(), count the usage between process creation and the first Update() which see it.

This is especially useful for short-lived processes & not too fast gathering (like process lifetime of 1 minutes and metric gathering every 10 seconds), where I had about 15% error in my case:

I'm measuring CPU usage of a PostgreSQL server behind a pgbouncer configured to keep connection 60 seconds (which means that every 60 seconds, the connection is closed & re-opened, which on Postgres means a process is terminated and a new one is re-created).

The real CPU usage is measured using ps --cumulative on parent process (which is a good approximation on long term, and a correct minimal value. Using this does not account CPU usage of currently running child but do count exactly CPU usage of all terminated children).

Without this PR, I have the following delta (over 1h30):

  • CPU seconds between two ps: 77 seconds
  • CPU seconds between two namedprocess_namegroup_cpu_seconds_total (both user + system): 65.2 seconds
  • Error: 11.80 seconds or 15%

Without this PR, I have the following delta (over 30 minutes):

  • CPU seconds between two ps: 28 seconds
  • CPU seconds between two namedprocess_namegroup_cpu_seconds_total (both user + system): 28.04 seconds
  • Error: none :)

When a process started between two Update(), count the usage between
process creation and the first Update() which see it.
@PierreF
Copy link
Contributor Author

PierreF commented Apr 14, 2020

Is there something missing for this PR to be merged ?

proc/tracker.go Outdated Show resolved Hide resolved
@ncabatoff
Copy link
Owner

Hi @PierreF,

The patch looks good, thanks very much! Sorry for the delay, I haven't worked on this project in quite a while, I needed to carve out some time to remember how it works so I could make sense of your change.

I made a minor suggestion to fix a typo in a comment, if you wouldn't mind fixing that I'll be happy to merge this.

Co-authored-by: ncabatoff <nick.cabatoff@gmail.com>
@PierreF
Copy link
Contributor Author

PierreF commented May 6, 2020

Sorry, missed the review. Fixed now!

@ncabatoff ncabatoff merged commit 9760f38 into ncabatoff:master May 16, 2020
@ncabatoff
Copy link
Owner

Thanks @PierreF !

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