* [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries @ 2025-08-08 15:08 Hannes Laimer 2025-08-11 9:25 ` Dominik Csapak 2025-08-27 9:05 ` [pve-devel] superseded: " Hannes Laimer 0 siblings, 2 replies; 4+ messages in thread From: Hannes Laimer @ 2025-08-08 15:08 UTC (permalink / raw) To: pve-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries 2025-08-08 15:08 [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries Hannes Laimer @ 2025-08-11 9:25 ` Dominik Csapak 2025-08-27 8:41 ` Hannes Laimer 2025-08-27 9:05 ` [pve-devel] superseded: " Hannes Laimer 1 sibling, 1 reply; 4+ messages in thread From: Dominik Csapak @ 2025-08-11 9:25 UTC (permalink / raw) To: pve-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries 2025-08-11 9:25 ` Dominik Csapak @ 2025-08-27 8:41 ` Hannes Laimer 0 siblings, 0 replies; 4+ messages in thread From: Hannes Laimer @ 2025-08-27 8:41 UTC (permalink / raw) To: pve-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] superseded: [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries 2025-08-08 15:08 [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries Hannes Laimer 2025-08-11 9:25 ` Dominik Csapak @ 2025-08-27 9:05 ` Hannes Laimer 1 sibling, 0 replies; 4+ messages in thread From: Hannes Laimer @ 2025-08-27 9:05 UTC (permalink / raw) To: pve-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-27 9:06 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-08-08 15:08 [pve-devel] [PATCH pve-cluster v2] fix #6615: rrd: only return populated entries Hannes Laimer 2025-08-11 9:25 ` Dominik Csapak 2025-08-27 8:41 ` Hannes Laimer 2025-08-27 9:05 ` [pve-devel] superseded: " Hannes Laimer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.