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

Breaking change: use state code rather than "description" #173

Closed
wants to merge 1 commit into from

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Mar 8, 2016

For process.Status(), linux is the only platform where it's possible to
get state descriptions. This is specific to the linux /proc/pid/status
file.

I think it would be better to get the more platform-neutral state codes,
which are also present on FreeBSD, OpenBSD, darwin, etc.

The state codes are also better documented on "man proc" and "man ps"

For process.Status(), linux is the only platform where it's possible to
get state descriptions. This is specific to the linux /proc/<pid>/status
file.

I think it would be better to get the more platform-neutral state codes,
which are also present on FreeBSD, OpenBSD, darwin, etc.

The state codes are also better documented on "man proc" and "man ps"
@shirou
Copy link
Owner

shirou commented Mar 8, 2016

Hmm. State codes are not common between platforms, I think.

For example,
U Marks a process in uninterruptible wait. in Darwin,
D Marks a process in disk (or other short term, uninterruptible) wait , in FreeBSD,
D uninterruptible sleep (usually IO) in Linux, from man ps.

Darwin also has L The process has pages locked in core (for example, for raw I/O). I don't know yet it is equal to Linux's D, but it may be.

I had gave up to standardize it, but if you know some pointers to the differences between platform. I will implement it.

@sparrc
Copy link
Contributor Author

sparrc commented Mar 8, 2016

R & S are pretty much standard

as far as I can tell, D == D on freebsd and linux, I think you can consider those both "blocked" or "uninterruptible sleep".

Darwin having U is a little odd, but it's also the only platform that uses U for any state, so it's easy to deal with

@sparrc
Copy link
Contributor Author

sparrc commented Mar 8, 2016

you can see how collectd classifies them in Linux here: https://github.com/collectd/collectd/blob/master/src/processes.c#L1871

keep scrolling down to see how it maps other states to "blocked"

@shirou
Copy link
Owner

shirou commented Mar 8, 2016

I check 3 platform man ps.

Linux FreeBSD Darwin  meaning
  R      R      R     running
  S      S      S     sleeping in an interruptible wait (sleeping less than about 20 seconds)
 none    I      I     idle (sleeping for longer than about 20 seconds)
  D      D      U     waiting in uninterruptible disk sleep
  Z      Z      Z     zombie
  W      *1     *2    paging
  T      T      T     traced or stopped

*1 FreeBSD has W but it means `Marks an idle interrupt thread.`
*2 Darwin has additional characters to indicate `W (paging)`

R is common. But, Sleep is not common (FreeBSD and Darwin has also I), and Paging is in the addition characters.

So, one approach is standardize as follows,

  • R (Running)
  • S (Sleep or Idle)
  • D (blocked)
  • W (paging)
  • Z (zombie)
  • T (trace or stopped)

FreeBSD and Darwin should have a convert code. How do you think?

@sparrc
Copy link
Contributor Author

sparrc commented Mar 8, 2016

I don't think that it needs to be normalized, why combine Idle for darwin and BSD? I think the gopsutil user can switch on the operating system to parse the state codes themselves.

The purpose behind the PR is to give the user access to the state codes, which are easier to parse than the somewhat arbitrary (and subject to change) descriptions available in /proc/pid/status

@sparrc
Copy link
Contributor Author

sparrc commented Mar 8, 2016

for example, in telegraf, I'd prefer to do this:

case 'W':
    switch runtime.GOOS{
    ...
    }
case 'U', 'D':
    // Also known as uninterruptible sleep or disk sleep
    blocked++
case 'Z':
    zombie++
case 'T':
    trace++
case 'R':
    running++
case 'S':
    sleeping++
case 'I':
    idle++

@shirou
Copy link
Owner

shirou commented Mar 8, 2016

lesseee, If you prefer platform-dependent , why not like this?

case "R", "running":
    running++
case "S", "sleeping":
    sleeping++

@sparrc
Copy link
Contributor Author

sparrc commented Mar 8, 2016

But there is a problem when there are additional parts to the states, for example, sometimes there are extra characters on the state:

R+
Ss
S+
S
S+
Ss

So this is actually the switch statement (we need to grab the 1st character):

switch status[0] {
    case 'W':
        switch runtime.GOOS{
        ...
        }
    case 'U', 'D':
        // Also known as uninterruptible sleep or disk sleep
        blocked++
    case 'Z':
        zombie++
    case 'T':
        trace++
    case 'R':
        running++
    case 'S':
        sleeping++
    case 'I':
        idle++
}

So we would need to grab the first character in some cases (when we have a state code), and in some cases not grab the first character (when we have a state description)

@shirou
Copy link
Owner

shirou commented Mar 8, 2016

OK, I agree to change the Linux status code. But as you write, it is a breaking change and it can not detect by compiling.

I opened #174, plan to introduce v2. I want to merge this PR at that time. How do you think?

@sparrc
Copy link
Contributor Author

sparrc commented Mar 9, 2016

that sounds good to me, thanks @shirou 😄

@shirou
Copy link
Owner

shirou commented Apr 12, 2016

This PR is included on v2. See #174. Thank you for your contirbution!

@shirou shirou closed this Apr 12, 2016
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