[pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries

[pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries

[Hannes Laimer]

RRD fetch can include the latest bucket before metrics are flushed,
resulting in entries with only a timestamp.

This now only returns entries with at least one value.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
since v1, thanks @Fabian:
 - just check count of keys in entry instead of keeping track of when
   something is added

 src/PVE/RRD.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
index 8fe1927..1813cdb 100644
--- a/src/PVE/RRD.pm
+++ b/src/PVE/RRD.pm
@@ -78,7 +78,8 @@ sub create_rrd_data {
                 # maybe make this configurable?
             }
         }
-        push @$res, $entry;
+        # 'time' + at least one metric
+        push @$res, $entry if (scalar keys %$entry) > 1;
     }
 
     return $res;
-- 
2.47.2



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

Re: [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries

[Dominik Csapak]

Hi,

please correct me if I'm wrong, but wouldn't this change drastically
change the graphs on the web ui ? (did not test it though)

when e.g. a storage is offline, it will have intermediary empty values?

so instead of having
[
{ time: 1, value: 1},
{ time: 2 },
{ time: 3, value: 2}
]

where we get a whole between 1 and 3, but with this patch we'd get

[
{ time: 1, value: 1},
{ time: 3, value: 2},
]

which would draw a line between 1 and 3 i guess?

On 8/8/25 17:07, Hannes Laimer wrote:
> RRD fetch can include the latest bucket before metrics are flushed,
> resulting in entries with only a timestamp.
> 
> This now only returns entries with at least one value.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> since v1, thanks @Fabian:
>   - just check count of keys in entry instead of keeping track of when
>     something is added
> 
>   src/PVE/RRD.pm | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
> index 8fe1927..1813cdb 100644
> --- a/src/PVE/RRD.pm
> +++ b/src/PVE/RRD.pm
> @@ -78,7 +78,8 @@ sub create_rrd_data {
>                   # maybe make this configurable?
>               }
>           }
> -        push @$res, $entry;
> +        # 'time' + at least one metric
> +        push @$res, $entry if (scalar keys %$entry) > 1;
>       }
>   
>       return $res;



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

Re: [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries

[Hannes Laimer]

On 11.08.25 11:25, Dominik Csapak wrote:
> Hi,
> 
> please correct me if I'm wrong, but wouldn't this change drastically
> change the graphs on the web ui ? (did not test it though)
> 
> when e.g. a storage is offline, it will have intermediary empty values?
> 
> so instead of having
> [
> { time: 1, value: 1},
> { time: 2 },
> { time: 3, value: 2}
> ]
> 
> where we get a whole between 1 and 3, but with this patch we'd get
> 
> [
> { time: 1, value: 1},
> { time: 3, value: 2},
> ]
> 
> which would draw a line between 1 and 3 i guess?
> 

It would, I didn't notice when I tested it.
Given this is only a problem for the last bucket, and only if read a few
seconds after a new one started. We can just check the last entry and
remove it if empty.

Thanks for pointing this out! I'll send a v2.

> On 8/8/25 17:07, Hannes Laimer wrote:
>> RRD fetch can include the latest bucket before metrics are flushed,
>> resulting in entries with only a timestamp.
>>
>> This now only returns entries with at least one value.
>>
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>> since v1, thanks @Fabian:
>>   - just check count of keys in entry instead of keeping track of when
>>     something is added
>>
>>   src/PVE/RRD.pm | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
>> index 8fe1927..1813cdb 100644
>> --- a/src/PVE/RRD.pm
>> +++ b/src/PVE/RRD.pm
>> @@ -78,7 +78,8 @@ sub create_rrd_data {
>>                   # maybe make this configurable?
>>               }
>>           }
>> -        push @$res, $entry;
>> +        # 'time' + at least one metric
>> +        push @$res, $entry if (scalar keys %$entry) > 1;
>>       }
>>       return $res;
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



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

[pve-devel] superseded: [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries

[Hannes Laimer]

superseded-by: 
https://lore.proxmox.com/pve-devel/20250827090431.48707-1-h.laimer@proxmox.com/T/#u

On 08.08.25 17:07, Hannes Laimer wrote:
> RRD fetch can include the latest bucket before metrics are flushed,
> resulting in entries with only a timestamp.
> 
> This now only returns entries with at least one value.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> since v1, thanks @Fabian:
>   - just check count of keys in entry instead of keeping track of when
>     something is added
> 
>   src/PVE/RRD.pm | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
> index 8fe1927..1813cdb 100644
> --- a/src/PVE/RRD.pm
> +++ b/src/PVE/RRD.pm
> @@ -78,7 +78,8 @@ sub create_rrd_data {
>                   # maybe make this configurable?
>               }
>           }
> -        push @$res, $entry;
> +        # 'time' + at least one metric
> +        push @$res, $entry if (scalar keys %$entry) > 1;
>       }
>   
>       return $res;



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