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 widget fails with ps from BusyBox #20

Open
xxxserxxx opened this issue Feb 15, 2020 · 10 comments
Open

Processes widget fails with ps from BusyBox #20

xxxserxxx opened this issue Feb 15, 2020 · 10 comments
Labels
enhancement New feature or request needs:question Further information is requested

Comments

@xxxserxxx
Copy link
Owner

xxxserxxx commented Feb 15, 2020

Required information:

  • gotop version (gotop -v): 2.0.1
  • The output of uname -a: Linux 4.2.8 Add ability to send different signals to processes #1 SMP Fri Dec 28 00:59:23 CST 2018 x86_64 GNU/Linux
  • Terminal emulator (e.g. iTerm or gnome terminal):
  • Any relevenat hardware info: QNAP QTS 4.3.6
  • tmux version if using tmux:

Logs :
"11:19:36 proc_linux.go:14: failed to retrieve processes: failed to execute 'ps' command: exit status 1"

On QNAP devices, ps command used Busybox version :

BusyBox v1.24.1 (2018-12-28 02:26:16 CST) multi-call binary.

Usage: ps [-o COL1,COL2=HEADER] [-T]

Show list of processes

        -o COL1,COL2=HEADER     Select columns for display
        -T                      Show threads

QNAP Options:
        --columns N

This version does not have "-axo" options so it fails. By default it shows processes of all users. Is it possible to have a version for BusyBox users ?
Thanks !

Original cjbassi#110
Submitter @Mikiya83

@xxxserxxx
Copy link
Owner Author

Busbox support sounds good to me. It would probably require us to do some check on the environment on start and adjust the ps flags. I'm curious what the adjustments would be. Is that actually the entirety of the man page? 😂 If not, could you copy the contents into a gist or link it?

So can you test this command: ps -o pid,comm,pcpu,pmem,args? Also, there's been some issues with the ps columns being cropped, so if that happens when running that, then you can try doing ps -o pid,comm=superlongheaderasdfasdfasdfsdaf etc.

@xxxserxxx
Copy link
Owner Author

Unfortunately it is the complete manpage :(

[~] # ps -h
ps: invalid option -- 'h'
BusyBox v1.24.1 (2018-12-28 02:26:16 CST) multi-call binary.

Usage: ps [-o COL1,COL2=HEADER] [-T]

Show list of processes

        -o COL1,COL2=HEADER     Select columns for display
        -T                      Show threads

QNAP Options:
        --columns N
[~] #

With yours commands i get :

ps -o pid,comm,pcpu,pmem,args 
ps: bad -o argument 'pcpu', supported arguments: user,group,comm,args,pid,ppid,pgid,etime,nice,rgroup,ruser,time,tty,vsz,stat,rss

I removed pcpu and pmem (because it's not recognized) and i get things like this :

[~] # ps -o pid,comm,args
  PID  COMMAND          Command
    1 init             init
    2 kthreadd         [kthreadd]
    3 ksoftirqd/0      [ksoftirqd/0]
    5 kworker/0:0H     [kworker/0:0H]
    7 rcu_sched        [rcu_sched]
...

I don't see any problems with cropping (i have some very long "Command" entries).

@xxxserxxx
Copy link
Owner Author

So it looks like ps on busybox doesn't support per process cpu usage or memory reporting. So we should check if we're on busybox and then our only option is to remove the unsupported flags and only show PID and command.

@xxxserxxx
Copy link
Owner Author

The only way i find to get cpu or memory reporting is to use top.
If you need other tests, i'm available ;)

@xxxserxxx
Copy link
Owner Author

Some new ideas about different ways to fix this:

  1. detect when a system uses busybox, then use cli args appropriate for ps on a busybox system, but we won't be able to get per-process cpu or memory usage
  2. require users to download and install procps on their system as a workaround
  3. directly gather process information ourselves by traversing /proc, or use the implementation provided by gopsutil

3 sounds best to me. We were originally using gopsutil to gather process information but I forget why we switched of it. And it should be easy to test gopsutil's implementation (I've just been busy).

@xxxserxxx xxxserxxx added bug Something isn't working enhancement New feature or request needs-info and removed bug Something isn't working needs-info labels Feb 19, 2020
@xxxserxxx
Copy link
Owner Author

Note to self:

docker run -it --name gotopbb --rm busybox
CGO_ENABLED=0 go build -a -v -o gotop ./cmd/gotop
docker cp ./gotop gotopbb:/bin/

@xxxserxxx
Copy link
Owner Author

xxxserxxx commented Feb 19, 2020

  • directly gather process information ourselves by traversing /proc, or use the implementation provided by gopsutil

3 sounds best to me. We were originally using gopsutil to gather process information but I forget why we switched of it. And it should be easy to test gopsutil's implementation (I've just been busy).

(Emphasis mine). Well, I found out why. CPU use quadruples under the gopsutil library vs repeatedly forking ps. Frankly, this surprises the heck out of me. However, changing to gopsutil does (a) remove the need for all of the OS-specific procs sources, and (b) enable procs to work properly on BusyBox.

I've filed shirou/gopsutil#842, and will leave this ticket open until a clear path forward is defined.

@Mikiya83 FYI

@xxxserxxx
Copy link
Owner Author

On a suggestion over in shirou/gopsutil#842 by @AtakanColak, I tried go-sysinfo. Despite eliminating the forking & parsing of output, and string manipulation and parsing to produce numbers, it's still twice as slow as forking ps. It's significantly faster than gopsutil, but still more CPU intensive than forking. I feel like I have to keep saying that, because it's so difficult to believe. In any case, it stays how it is on Linux, and I think the way forward is one of the suggestions above: detect when gotop is running in busybox and use go-sysinfo there.

@xxxserxxx
Copy link
Owner Author

Gory details are in shirou/gopsutil#842, but the summary is: a benchmark shows that elastic/go-sysinfo is marginally faster than the current forking/parsing code. It doesn't explain why top showed that version as having higher CPU use, but I'm willing to give it a shot if it means supporting more environments.

@xxxserxxx
Copy link
Owner Author

Another hiccup: go-sysinfo fails to get information about many processes when run as a user because it doesn't have access to all processes in /proc. This may be insurmountable.

@xxxserxxx xxxserxxx added needs:question Further information is requested and removed needs:info labels Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs:question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant