[pve-devel] [PATCH pve-container] pct: fix cpu load calculation on command line

[Philipp Hufnagl]

Hello

On 7/18/23 15:02, Thomas Lamprecht wrote:
> Am 18/07/2023 um 14:00 schrieb Philipp Hufnagl:
>> Sorry forgott to tag as v2
> and also forgot to document the patch changelog like asked yesterday..
Sorry. I did not know that. I will add a changelog
>
>> On 7/18/23 13:58, Philipp Hufnagl wrote:
>>>    When called from the command line, it was not possible to calculate
>>>    cpu load because there was no 2nd data point available for the
>>>    calculation. Now (when called) from the command line, cpu stats will
>>>    be fetched twice with a minimum delta of 20ms. That way the load can
>>>    be calculated
> @Maximiliano, didn't we decide to just drop it instead? This isn't really
> useful, once can get much better data from the pressure stall information
> (PSI) which is tracked per cgroup and tells a user much more than a 20 ms
> sample interval..
>
> https://docs.kernel.org/accounting/psi.html#cgroup2-interface
>
> Still a few comments inline.

Shall I wait with a v3 until a decision is made?

> ust drop this CPU load stuff in pct status I'd rather do one of four
> options:
>
> 1) rename this to prime_vmstatus_cpu_sampling and just do it for a single vmid,
> then call this new method in PVE::CLI::pct->status and do the sleep there, as
> that's actually the one call sites that cares about it, the existing vmstatus
> method then just needs one change:
>
> -           if ($delta_ms > 1000.0) {
> +           if ($delta_ms > 1000.0 || $old->{cpu} == 0) {
>
> 2) The same as 1) but instead of adding the prime_vmstatus_cpu_sampling helper
> just call vmstatus twice with sleeping in-between (and the same change to the if
> condition as for 1).
>
> 3) get the data where it's already available, i.e., pvestatd, might need more
> rework though
>
> 4) switch over to reporting the PSI from /sys/fs/cgroup/lxc/VMID/cpu.pressure
> this is pretty simple as in PSI ~ 0 -> no overload 0 >> PSI > 1 -> some overload
> and PSI >> 1 a lot of overload.
>
> Option 4 sounds niceish, but needs more work and has not that high of a benefit
> (users can already query this easily themselves), option 1 or 2 would be OK-ish,
> but IMO not ideal, as we'd use a 20ms avg here compared to a >> 1s average elswhere,
> which can be confusing as it can be quite, well spikey. option 3 would be better here
> but as mentioned also more rework and possible more intrusive one, so IMO just
> dropping it sounds almost the nicest and def. most simple one.
>
I think we should do Idea 1 as a solution until we finish a deeper rework