Repository navigation
fix(processlist): index io_counters instead of testing membership in it - #3675
Merged
Merged
Conversation
`_sum_stats(key, sub_key)` guarded the accumulation with `sub_key in p[key]`.
That is a mapping test, and `sub_key` is only a mapping key for `memory_info`.
For `io_counters` it is an index — the value is the list
[read_bytes, write_bytes, read_bytes_old, write_bytes_old, io_tag]
so `0 in p['io_counters']` asked whether the counters *contain the number zero*.
A process doing real IO was skipped, and one whose counters happened to hold the
literal 0/1/2/3 was summed instead:
[[5000, 700, 2000, 300, 1], _sum_stats('io_counters', 0) -> 0
[3000, 400, 1000, 200, 1]] (expected 8000)
[[5000, 700, 0, 300, 1]] _sum_stats('io_counters', 0) -> 5000
(summed only because 0 is in the list)
So the R/s and W/s totals on the process-list summary row — rendered whenever a
process filter is active — were not merely zero but arbitrary. The VIRT and RES
totals beside them were correct, because `memory_info` really is a mapping.
Branch on the container type, and bounds-check the index so a short list is
skipped rather than raising.
Owner
|
Thanks for the PR @ntdatt812 Merged into develop (i solve a conflict in the test file). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
_sum_stats(key, sub_key)(glances/plugins/processlist/__init__.py:1030) guards the accumulation with a mapping test:sub_keyis only a mapping key formemory_info. Forio_countersit is an index — the value is the list[read_bytes, write_bytes, read_bytes_old, write_bytes_old, io_tag](glances/processes.py:553) — so0 in p['io_counters']asks whether the counters contain the number zero.Measured on
develop:The symptom
The
R/sandW/sfigures on the process-list summary row —_msg_curse_sum, rendered whenever a process filter is active, for current/min/max — are computed as_sum_stats(…, 0) - _sum_stats(…, sub_key=2)at:945and:955. Both terms come back 0 for every process actually doing IO, so the row reads zero; and when some idle process happens to carry the literal index in its counters, it reads a number that belongs to nobody. TheVIRT/REStotals on the same row are right, which is what makes the zero look plausible.The fix
Branch on the container type, and bounds-check the index so a short list is skipped rather than raising
IndexError. The mapping path is untouched.Verification
Windows 11, Python 3.14.7.
Mutation-checked, after confirming the edit applied: disabling the new branch fails the new case.
Tests
tests/test_core.py::test_010c_processlist_sum_stats_indexes_lists— all fourio_countersindexes against known values, the mapping path unchanged (including an absent key), the "list contains the literal index" case that must no longer decide anything, and an out-of-range index that must be skipped rather than raise.Note
The same shape exists at
glances/folder_list.py:122,135(i in self.timer_folders, an int against a list ofTimers, so the per-folderfolder_N_refreshoption never takes effect). I left it out of this PR since it is a different plugin with a different symptom — happy to send it separately if useful.