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

Fix top_mem plugin #22

Merged
merged 1 commit into from
Jun 3, 2023

Conversation

jalopezg-git
Copy link
Contributor

ISSUE TYPE
  • Bugfix pull-request
DSTAT VERSION

Dool 1.0.0
Written by Scott Baker scott@perturb.org
Forked from Dstat written by Dag Wieers dag@wieers.com
Homepage at https://github.com/scottchiefbaker/dool/

Platform posix/linux
Kernel 5.16.8-arch1-1
Python 3.10.2 (main, Jan 15 2022, 19:56:27) [GCC 11.1.0]

Terminal type: xterm (color support)
Terminal size: 24 lines, 80 columns

Processors: 32
Pagesize: 4096
Clock ticks per secs: 100

internal:
aio,cpu,cpu-adv,cpu-use,cpu24,disk,disk24,disk24-old,epoch,
fs,int,int24,io,ipc,load,lock,mem,mem-adv,net,page,page24,
proc,raw,socket,swap,swap-old,sys,tcp,time,udp,unix,vm,
vm-adv,zones
/tmp/dool/plugins:
battery,battery-remain,condor-queue,cpufreq,dbus,disk-avgqu,
disk-avgrq,disk-svctm,disk-tps,disk-util,disk-wait,dool,
dool-cpu,dool-ctxt,dool-mem,fan,freespace,fuse,gpfs,gpfs-ops,
helloworld,ib,innodb-buffer,innodb-io,innodb-ops,jvm-full,
jvm-vm,lustre,md-status,memcache-hits,mongodb-conn,mongodb-mem,
mongodb-opcount,mongodb-queue,mongodb-stats,mysql-io,mysql-keys,
mysql5-cmds,mysql5-conn,mysql5-innodb,mysql5-innodb-basic,
mysql5-innodb-extra,mysql5-io,mysql5-keys,net-packets,nfs3,
nfs3-ops,nfsd3,nfsd3-ops,nfsd4-ops,nfsstat4,ntp,postfix,power,
proc-count,qmail,redis,rpc,rpcd,sendmail,snmp-cpu,snmp-load,
snmp-mem,snmp-net,snmp-net-err,snmp-sys,snooze,squid,test,
thermal,top-bio,top-bio-adv,top-childwait,top-cpu,top-cpu-adv,
top-cputime,top-cputime-avg,top-int,top-io,top-io-adv,
top-latency,top-latency-avg,top-mem,top-oom,utmp,vm-cpu,vm-mem,
vm-mem-adv,vmk-hba,vmk-int,vmk-nic,vz-cpu,vz-io,vz-ubc,wifi,
zfs-arc,zfs-l2arc,zfs-zil

SUMMARY

Usage of proc_splitline() breaks for processes containing spaces as part of the command name, e.g.

2566 (tmux: client) S 2459 2566 2459 34817 2566 4194304 290 14 12 0 0 0 0 0 23 3 1 0 124899 11952128 1010 18446744073709551615 94216560173056 94216560738917 140737080092208 0 0 0 0 3674116 134433283 1 0 0 17 14 0 0 0 0 0 94216560947760 94216561019336 94216574795776 140737080101018 140737080101023 140737080101023 140737080102890 0

which ends up taking the value of some other field instead of the RSS.

Given that the command name is only used if it is a new topper, avoid the overhead of regex matching and use /proc/[pid]/statm instead (RSS is the second field there).
For the (rare) case in which we need to refresh the process name, get it from /proc/[pid]/comm.

$ ./dool --top-mem # before
--most-expensive-
  memory process
tmux     9677G

$ ./dool --top-mem # after
--most-expensive-
  memory process
firefox      427M

Usage of `proc_splitline()` breaks for processes containing spaces as part of the command name, e.g.
```
2566 (tmux: client) S 2459 2566 2459 34817 2566 4194304 290 14 12 0 0 0 0 0 23 3 1 0 124899 11952128 1010 18446744073709551615 94216560173056 94216560738917 140737080092208 0 0 0 0 3674116 134433283 1 0 0 17 14 0 0 0 0 0 94216560947760 94216561019336 94216574795776 140737080101018 140737080101023 140737080101023 140737080102890 0
```
which ends up taking the value of some other field instead of the RSS.

Given that the command name is only used if it is a new topper, avoid using a regex and use
`/proc/[pid]/statm` instead.
@jalopezg-git
Copy link
Contributor Author

Ping!

@jalopezg-git
Copy link
Contributor Author

jalopezg-git commented Oct 10, 2022

@scottchiefbaker Gentle reminder on the fix proposed in this PR. 🙂

@wakamex
Copy link

wakamex commented Dec 28, 2022

I came to the same conclusion, wish I'd seen this first. or that this had been merged.

@scottchiefbaker
Copy link
Owner

I have no idea how I never got notifications on this... I just wandered across it. Apologies.

@scottchiefbaker scottchiefbaker merged commit ac8bc66 into scottchiefbaker:master Jun 3, 2023
@jalopezg-git jalopezg-git deleted the fix-top-mem branch June 3, 2023 23:28
@jalopezg-git
Copy link
Contributor Author

I have no idea how I never got notifications on this... I just wandered across it. Apologies.

No worries!

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