Re: [pve-devel] [PATCH manager v3 06/14] ui: rrdmodels: add new columns and update existing

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline:

On 7/15/25 16:32, Aaron Lauterer wrote:
> Memory columns will be used in an area graph, which cannot handle
> gaps directly. Therefore we set the default value to 'null'. This makes
> it easier to handle the tooltip when there is no data.
> 
> We calculate memused to subtract the arcsize. While the columns report
> what they represent, in the stacked/area memory graph of the node we
> need to account for the fact that memused includes the ZFS arc.
> 
> We also calculate the total / free memory for nodes and guests to get
> the diff from the configured/max memory from the current used one.
> Otherwise, we might see some spikes in the overall memory graph,
> as the can be small timing differences when the data is collected.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> Notes:
>      changes since:
>      v2:
>      * add default values where needed for area graphs
>      * add calculated values, usually to keep the memory graphs from spiking
>        at the top due to slight timing differences where the data doesn't
>        align perfectly
>      RFC:
>      * drop node memcache and membuffer columns as we now have memavailable
>        which is better suited
> 
>   www/manager6/data/model/RRDModels.js | 44 ++++++++++++++++++++++++++--
>   1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/www/manager6/data/model/RRDModels.js b/www/manager6/data/model/RRDModels.js
> index 82f4e5cd..70b45986 100644
> --- a/www/manager6/data/model/RRDModels.js
> +++ b/www/manager6/data/model/RRDModels.js
> @@ -18,14 +18,39 @@ Ext.define('pve-rrd-node', {
>           'loadavg',
>           'maxcpu',
>           'memtotal',
> -        'memused',
> +        { name: 'memused', defaultValue: null },
>           'netin',
>           'netout',
>           'roottotal',
>           'rootused',
>           'swaptotal',
>           'swapused',
> +        { name: 'memfree', defaultValue: null },
> +        { name: 'arcsize', defaultValue: null },
> +        'pressurecpusome',
> +        'pressureiosome',
> +        'pressureiofull',
> +        'pressurememorysome',
> +        'pressurememoryfull',
>           { type: 'date', dateFormat: 'timestamp', name: 'time' },
> +        {
> +            name: 'memfree-capped',
> +            calculate: function (data) {
> +                if (data.memtotal === null || data.memused === null) {
> +                    return null;
> +                }
> +                return data.memtotal - data.memused;

i think if you did the check in reverse, you can omit the 'defaultValue: 
null'

by e.g. doing something like:

if (data.memtotal >= 0 && data.memused >= 0 && data.memtotal >= 
data.memused) {
return data.memtotal - data.memused;
}

return null;

?

> +            },
> +        },
> +        {
> +            name: 'memused-sub-arcsize',
> +            calculate: function (data) {
> +                if (data.memused === null) {
> +                    return null;
> +                }
> +                return data.memused - data.arcsize;

here you only check memused for null. what about data.arcsize?

> +            },
> +        },
>       ],
>   });
>   
> @@ -42,13 +67,26 @@ Ext.define('pve-rrd-guest', {
>           'maxcpu',
>           'netin',
>           'netout',
> -        'mem',
> -        'maxmem',
> +        { name: 'mem', defaultValue: null },
> +        { name: 'maxmem', defaultValue: null},
>           'disk',
>           'maxdisk',
>           'diskread',
>           'diskwrite',
> +        {name: 'memhost', defaultValue: null},
> +        'pressurecpusome',
> +        'pressurecpufull',
> +        'pressureiosome',
> +        'pressurecpufull',
> +        'pressurememorysome',
> +        'pressurememoryfull',
>           { type: 'date', dateFormat: 'timestamp', name: 'time' },
> +        {
> +            name: 'maxmem-capped',
> +            calculate: function (data) {
> +                return data.maxmem - data.mem;

here you don't check neither maxmem nor mem for 'null'

> +            },
> +        },
>       ],
>   });
>   



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel