processes: Show real disk IO in addition to process IO (Linux only) #2232
Conversation
|
This looks good to me 👍 The only "breaking change" is that |
There was a problem hiding this comment.
Hi @rpv-tomsk, thank you very much for your patch! I think it is reasonable, but should be part of a feature release, not a patch release.
The ps_ prefix of the types used by the processes plugin are an annoying legacy and I'd prefer not to introduce additional types with that prefix. Could we please use the following mapping instead:
io_rchar/io_wchar -> io_octets (renamed from ps_disk_octets)
io_syscr/io_syscw -> io_ops (renamed from ps_disk_ops)
read_bytes/write_bytes -> ps_disk_octets (new data collected to existing type)
Where io_ops is a new type and io_octets is an existing type. I'm a bit undecided between using ps_disk_octets (don't break existing metrics) and switching to disk_octets (data will be different, might as well use a new type). What do you think?
The code itself is flawless. Thank you! :-D
—octo
|
If the data is different, shouldn't a new name be used anyway, to make sure that it won't be confused with old data? Then a new type can be used, too. |
143422b to
7c6486a
Compare
|
Types renamed. |
Before this patch, Collectd reports /proc/PID/io fields 'io_rchar/io_wchar' as 'ps_disk_octets' type and 'io_syscr/io_syscw' as 'ps_disk_ops' type. The names of these types do not match values they represent. New, correct mapping implemented: io_rchar/io_wchar -> io_octets (was ps_disk_octets, not used anymore) io_syscr/io_syscw -> io_ops (was ps_disk_ops, not used anymore) read_bytes/write_bytes -> disk_octets (new data collected) Closes: collectd#1990
Collectd reports
/proc/PID/iofieldsio_rchar/io_wcharasps_disk_octetstype andio_syscr/io_syscwasps_disk_opstype.The names of these types do not match values they represent.
For
io_rchar/io_wchardata type should be named asps_io_octets. In Linux, these values represent any I/O of process, not disk I/O only: they include sockets, pipes, console I/O, etc.The
ps_io_octetstype can be added in addition to the existingps_disk_octetsfor which values can be obtained fromread_bytes/write_bytesof /proc/PID/io. These values will be much more correct for reporting asps_disk_octetsthan existingio_rchar/io_wchar. Please note whatread_bytes/write_bytesis real disk I/O, it does not account file I/O from cache.For
io_syscr/io_syscwcorrect type should be namedps_io_ops.Reported values do not match any real disk ops.
My proposal is to fix these issues.
New, correct mapping implemented:
Related documentation: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/proc.txt?id=HEAD#n1558
Issues: #1990, #1225