diff --git a/glances/plugins/processlist/model.py b/glances/plugins/processlist/model.py index 751cbd4d01..dac06cce72 100644 --- a/glances/plugins/processlist/model.py +++ b/glances/plugins/processlist/model.py @@ -189,9 +189,10 @@ def __init__(self, store: StatsStore, config: GlancesConfig) -> None: # publishing it whole would duplicate a dozen fields already in `data[]` # for the same pid. # - # `cmdline` titles the block, with its arguments, on both surfaces. The - # engine merges it into the process after the extended grab, so it copies - # it into `extended_process` separately (`GlancesProcesses.update`). + # `cmdline` titles the block, with its arguments, on both surfaces. It + # reaches `extended_process` with the rest of the process dict — the + # engine merges the cached attrs before the extended grab, in the + # pre-sort metadata pass (`GlancesProcesses.update`). _EXTENDED_KEYS: ClassVar[tuple[str, ...]] = ( "pid", "name", diff --git a/glances/processes.py b/glances/processes.py index 2f05237f52..5833022752 100644 --- a/glances/processes.py +++ b/glances/processes.py @@ -684,16 +684,39 @@ def update(self): # Build the process list processlist = self.build_process_list(sorted_attrs) + # Update the processcount (on the raw status strings: the metadata + # pass below truncates 'status' to its first letter for display) + self.update_processcount(processlist) + + # Loop over the processes to materialize the fields a sort key can + # point at, BEFORE sorting. `io_counters` is still a psutil namedtuple + # here — indexes 2/3 are read/write *counts*, not the previous-cycle + # byte totals the sort lambda subtracts — and `cpu_times` a pcputimes + # namedtuple with no .get(), which made the sort raise and silently + # fall back to cpu_percent order (issue #3830). The cached attrs + # (username, cmdline) only exist on each proc after this pass as well. + for proc in processlist: + # Meta data + ########### + proc = self.get_pid_time_and_status(time_since_update, proc) + + # Process IO + proc = self.get_io_counters(proc) + + # Manage cached information + proc = self.maybe_add_cached_stats(is_cached, cached_attrs, proc) + + # cpu_times reaches the sort (and consumers) as a dict, the shape + # update_list produces with namedtuple_to_dict + cpu_times = proc.get('cpu_times') + if hasattr(cpu_times, '_asdict'): + proc['cpu_times'] = cpu_times._asdict() + # Sort the processes list by the current sort_key # This is needed so that cursor_position matches the displayed order (see issue #3400) processlist = sort_stats(processlist, sorted_by=self.sort_key, reverse=self.sort_reverse) - # Update the processcount - self.update_processcount(processlist) - - # Loop over processes and : - # - add extended stats for selected process - # - add metadata + # Loop over processes and add extended stats for the selected process # Whether the pinned process was still running this cycle. Without it, # a process that exits leaves `extended_process` holding its last # values forever -- nothing below ever runs again for a pid that is no @@ -719,21 +742,6 @@ def update(self): self.extended_process = namedtuple_to_dict(proc) extended_seen = True - # Meta data - ########### - proc = self.get_pid_time_and_status(time_since_update, proc) - - # Process IO - proc = self.get_io_counters(proc) - - # Manage cached information - proc = self.maybe_add_cached_stats(is_cached, cached_attrs, proc) - - # The command line is only merged above, after the extended grab: - # give it to the pinned process so its title can show it. - if self.extended_process is not None and proc['pid'] == self.extended_process['pid']: - self.extended_process['cmdline'] = proc.get('cmdline') - # The pinned process is gone. Forget it, rather than leave the TUI and # the WebUI showing its last numbers under a `extended_stats: True` # that is no longer true. The PIN goes with the accumulator: a pin on diff --git a/tests/test_processes_extended.py b/tests/test_processes_extended.py index 2f12fe3be5..1251379ca4 100644 --- a/tests/test_processes_extended.py +++ b/tests/test_processes_extended.py @@ -236,8 +236,9 @@ def test_a_new_pin_starts_from_fresh_min_max_mean(monkeypatch): def test_the_pinned_process_carries_its_command_line(monkeypatch): - """`cmdline` is merged into the process after the extended grab; the - pinned process must still carry it, as it titles the block.""" + """`cmdline` is merged into the process before the extended grab, in the + pre-sort metadata pass; the pinned process must still carry it, as it + titles the block.""" import os pid = os.getpid() diff --git a/tests/test_processes_sort_order.py b/tests/test_processes_sort_order.py new file mode 100644 index 0000000000..226542c6dc --- /dev/null +++ b/tests/test_processes_sort_order.py @@ -0,0 +1,127 @@ +"""`update()` must sort the process list on post-conversion values. + +`build_process_list` returns raw `as_dict()` rows: `cpu_times` is a +``pcputimes`` namedtuple and `io_counters` a ``pio`` one — psutil's +``pio(read_count, write_count, read_bytes, write_bytes, ...)`` puts the +*counts* at indexes 0/1, which is why `get_io_counters` reads bytes at +indexes 2/3. The engine used to sort that raw list before the per-process +loop ran `get_io_counters` / `namedtuple_to_dict`, so +`--sort-processes io_counters` subtracted the byte totals as if they were +the previous cycle's values, and `cpu_times` sorting raised inside +`sorted()` and silently fell back to `cpu_percent` order (issue #3830). +The REST API and WebUI serve this engine order directly (`get_list()`); +only the TUI re-sorted later. +""" + +from collections import namedtuple + +import pytest + +from glances.processes import GlancesProcesses + +# Raw psutil shapes, as `p.as_dict()` produces them. +pcputimes = namedtuple('pcputimes', ['user', 'system', 'children_user', 'children_system']) +pio = namedtuple('pio', ['read_count', 'write_count', 'read_bytes', 'write_bytes']) + + +def _raw_proc(pid, name, cpu_percent, cpu_times, io_counters, memory_percent=1.0): + """A process row in the shape `build_process_list` emits it (pre-conversion).""" + return { + 'pid': pid, + 'name': name, + 'cmdline': [name], + 'cpu_percent': cpu_percent, + 'memory_percent': memory_percent, + 'memory_info': {'rss': 0, 'vms': 0}, + 'status': 'sleeping', + 'num_threads': 1, + 'nice': 0, + 'username': 'root', + 'cpu_times': cpu_times, + 'io_counters': io_counters, + 'gids': {'real': 0}, + } + + +@pytest.fixture +def engine(): + return GlancesProcesses() + + +def test_sort_by_cpu_times_orders_on_user_plus_system(engine, monkeypatch): + # The busiest-by-time process has the LOWEST cpu_percent: a fallback to + # cpu_percent order (what the pre-fix sort silently did) inverts the list. + rows = [ + _raw_proc(1, 'highest-cpu', 99.0, pcputimes(1.0, 0.0, 0.0, 0.0), None), + _raw_proc(2, 'long-runner', 1.0, pcputimes(500.0, 40.0, 0.0, 0.0), None), + _raw_proc(3, 'mid', 5.0, pcputimes(100.0, 10.0, 0.0, 0.0), None), + ] + monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows) + engine.set_sort_key('cpu_times', auto=False) + + engine.update() + + assert [p['name'] for p in engine.processlist] == ['long-runner', 'mid', 'highest-cpu'] + + +def test_sort_by_io_counters_orders_on_bytes_since_last_cycle(engine, monkeypatch): + # The writer performs many small writes: its write *count* dwarfs nothing, + # but its byte totals are huge — sorting the raw namedtuple subtracts the + # bytes as if they were last cycle's, sending it to the bottom (the + # reported symptom). + rows = [ + _raw_proc(1, 'idle-reader', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(50, 40, 50, 500)), + _raw_proc(2, 'heavy-writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 90000, 100, 10_000_000)), + _raw_proc(3, 'no-io', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), None), + ] + monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows) + engine.set_sort_key('io_counters', auto=False) + + engine.update() + + assert [p['name'] for p in engine.processlist] == ['heavy-writer', 'idle-reader', 'no-io'] + + +def test_sort_by_io_counters_uses_the_rate_not_lifetime_bytes(engine, monkeypatch): + # Second cycle: 'writer' added few new bytes while 'bursty' added many. + # Only post-conversion rows (with the previous cycle's bytes merged in) + # can order by rate. + cycle = iter( + [ + [ + _raw_proc(1, 'writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 10_000_000)), + _raw_proc(2, 'bursty', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 1_000)), + ], + [ + _raw_proc(1, 'writer', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 10_100_000)), + _raw_proc(2, 'bursty', 0.0, pcputimes(0.0, 0.0, 0.0, 0.0), pio(1, 10, 0, 5_000_000)), + ], + ] + ) + monkeypatch.setattr(engine, 'build_process_list', lambda attrs: next(cycle)) + engine.set_sort_key('io_counters', auto=False) + + engine.update() + assert [p['name'] for p in engine.processlist] == ['writer', 'bursty'] + + engine.update() + assert [p['name'] for p in engine.processlist] == ['bursty', 'writer'] + + +def test_engine_order_matches_a_post_conversion_resort(engine, monkeypatch): + """The API serves `get_list()` unsorted: the engine order must already be + what `get_list(sorted=True)` — the post-conversion re-sort — produces.""" + rows = [ + _raw_proc(1, 'a', 50.0, pcputimes(1.0, 1.0, 0.0, 0.0), pio(5, 5, 10, 10)), + # Biggest lifetime writer: raw-order (count - bytes) demotes 'b' last. + _raw_proc(2, 'b', 1.0, pcputimes(300.0, 0.0, 0.0, 0.0), pio(5, 9000, 10, 10_000_000)), + _raw_proc(3, 'c', 10.0, pcputimes(20.0, 5.0, 0.0, 0.0), pio(5, 50, 10, 40)), + ] + monkeypatch.setattr(engine, 'build_process_list', lambda attrs: rows) + engine.set_sort_key('io_counters', auto=False) + + engine.update() + api_order = [p['name'] for p in engine.get_list()] + resorted = [p['name'] for p in engine.get_list(sorted=True)] + + assert api_order == resorted == ['b', 'c', 'a']