all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH cluster 2/3] RRD: fetch data from old rrd file if present and needed
Date: Fri,  5 Sep 2025 15:55:12 +0200	[thread overview]
Message-ID: <20250905135517.4005478-3-a.lauterer@proxmox.com> (raw)
In-Reply-To: <20250905135517.4005478-1-a.lauterer@proxmox.com>

One side effect of the RRD migration is that MAX spikes are flattened
due to the new more fine grained resolution.

If we request RRD data that was still covered in the old rrd file, we
can fetch those data from there and then append newer data from the new
RRD files. This way, we still have the older more coarse resolution for
the old data and keep spikes present.

Old RRD files can be either in {rrd}.old or just {rrd} without the .old
suffix. This depends on if the rrd migration tool has been run on the
host or not.

One side effect of this more dynamic approach is that the step size
determined by RRDs::fetch will not match up in many situations.
Therefore we drop the check for the step size!

For example, if we choose Year as the time frame, but the new file only
has a much shorter amound of data present, RRDs::fetch will use a
smaller step size, if it can still cover the requested data with it.

Visually this will result in very small step sizes, that should get
wider once we get more or all data from the new RRD files, where it
needs to then switch to the longer step sizes to cover the requested
time frame.

For the PNG generation we rely on rrds graphing functionality which can
also handle multiple data lines. With the way we generate the data line from
the old line, it should look like one line.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/PVE/RRD.pm | 154 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 119 insertions(+), 35 deletions(-)

diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
index 932191d..42e0049 100644
--- a/src/PVE/RRD.pm
+++ b/src/PVE/RRD.pm
@@ -7,6 +7,32 @@ use RRDs;
 
 use PVE::Tools;
 
+my $get_rrd_data = sub {
+    my ($rrd, $cf, $is_node, $reso, $args, $res) = @_;
+    my ($start, $step, $names, $data) = RRDs::fetch($rrd, $cf, @$args);
+
+    my $err = RRDs::error;
+    die "RRD error: $err\n" if $err;
+
+    my $fields = scalar(@$names);
+    for my $line (@$data) {
+        my $entry = { 'time' => $start };
+        $start += $step;
+        for (my $i = 0; $i < $fields; $i++) {
+            my $name = $names->[$i];
+            if (defined(my $val = $line->[$i])) {
+                $entry->{$name} = $val;
+                $entry->{memavailable} = $val
+                    if $is_node && $name eq 'memfree' && !exists($entry->{memavailable});
+            } else {
+                # leave empty fields undefined
+                # maybe make this configurable?
+            }
+        }
+        push @$res, $entry;
+    }
+};
+
 sub create_rrd_data {
     my ($rrdname, $timeframe, $cf) = @_;
 
@@ -33,54 +59,71 @@ sub create_rrd_data {
         decade => [86400 * 7, 570], # 1 week resolution, 10 years
     };
 
+    my $is_node = !!($rrdname =~ /^pve-node/);
+    $cf = "AVERAGE" if !$cf;
+    my $res = [];
+
     if ($rrdname =~ /^pve2/) {
         $setup = $setup_pve2;
         $timeframe = "year" if $timeframe eq "decade"; # we only store up to one year in the old format
     }
-    my $is_node = !!($rrdname =~ /^pve-node/);
 
     my ($reso, $count) = @{ $setup->{$timeframe} };
     my $ctime = $reso * int(time() / $reso);
     my $req_start = $ctime - $reso * $count;
 
-    $cf = "AVERAGE" if !$cf;
-
-    my @args = (
-        "-s" => $req_start,
-        "-e" => $ctime - 1,
-        "-r" => $reso,
-    );
-
-    my $socket = "/var/run/rrdcached.sock";
-    push @args, "--daemon" => "unix:$socket" if -S $socket;
-
-    my ($start, $step, $names, $data) = RRDs::fetch($rrd, $cf, @args);
-
-    my $err = RRDs::error;
-    die "RRD error: $err\n" if $err;
-
-    die "got wrong time resolution ($step != $reso)\n"
-        if $step != $reso;
+    my $last_old;
+    # check if we have old rrd file and if the start point is still covered by
+    # it, fetch that data from it for any data not available in the old file we
+    # will fetch it from the new file.
+    if ($rrdname =~ /pve-(?<type>node|vm|storage)-[0-9]*\.[0-9]*\/(?<resource>.*)/) {
+        my $old_rrd = "${rrddir}/pve2-$+{type}/$+{resource}";
+        my $old_exists = 0;
+
+        # we can have already migrated rrd files that have the .old suffix too
+        if (-e $old_rrd) {
+            $old_exists = 1;
+        } elsif (-e "${old_rrd}.old") {
+            $old_exists = 1;
+            $old_rrd = "${old_rrd}.old";
+        }
 
-    my $res = [];
-    my $fields = scalar(@$names);
-    for my $line (@$data) {
-        my $entry = { 'time' => $start };
-        $start += $step;
-        for (my $i = 0; $i < $fields; $i++) {
-            my $name = $names->[$i];
-            if (defined(my $val = $line->[$i])) {
-                $entry->{$name} = $val;
-                $entry->{memavailable} = $val
-                    if $is_node && $name eq 'memfree' && !exists($entry->{memavailable});
+        if ($old_exists) {
+            $last_old = RRDs::last($old_rrd);
+            if ($req_start < $last_old) {
+                my ($reso_old, $count_old) = @{ $setup_pve2->{$timeframe} };
+                my $ctime_old = $reso_old * int(time() / $reso_old);
+                my $req_start_old = $ctime_old - $reso_old * $count_old;
+                my $args = [];
+                push(@$args, "-s" => $req_start_old);
+                push(@$args, "-e" => $last_old);
+                push(@$args, "-r" => $reso_old);
+
+                my $socket = "/var/run/rrdcached.sock";
+                push @$args, "--daemon" => "unix:$socket" if -S $socket;
+
+                $get_rrd_data->($old_rrd, $cf, $is_node, $reso_old, $args, $res);
             } else {
-                # leave empty fields undefined
-                # maybe make this configurable?
+                $last_old = undef;
             }
         }
-        push @$res, $entry;
     }
 
+    my $args = [];
+    if ($last_old) {
+        push(@$args, "-s" => $last_old);
+    } else {
+        push(@$args, "-s" => $req_start);
+    }
+
+    push(@$args, "-e" => $ctime - 1);
+    push(@$args, "-r" => $reso);
+
+    my $socket = "/var/run/rrdcached.sock";
+    push @$args, "--daemon" => "unix:$socket" if -S $socket;
+
+    $get_rrd_data->($rrd, $cf, $is_node, $reso, $args, $res);
+
     return $res;
 }
 
@@ -126,6 +169,33 @@ sub create_rrd_graph {
     }
 
     my ($reso, $count) = @{ $setup->{$timeframe} };
+    my $ctime = $reso * int(time() / $reso);
+    my $req_start = $ctime - $reso * $count;
+
+    my $last_old;
+    my $old_rrd;
+    my $old_exists = 0;
+    my $use_old;
+    # check if we have old rrd file and if the start point is still covered
+    # by it
+    if ($rrdname =~ /pve-(?<type>node|vm|storage)-[0-9]*\.[0-9]*\/(?<resource>.*)/) {
+        $old_rrd = "${rrddir}/pve2-$+{type}/$+{resource}";
+
+        # we can have already migrated rrd files that have the .old suffix too
+        if (-e $old_rrd) {
+            $old_exists = 1;
+        } elsif (-e "${old_rrd}.old") {
+            $old_exists = 1;
+            $old_rrd = "${old_rrd}.old";
+        }
+
+        if ($old_exists) {
+            $last_old = RRDs::last($old_rrd);
+            if ($req_start < $last_old) {
+                $use_old = 1;
+            }
+        }
+    }
 
     my @args = (
         "--imgformat" => "PNG",
@@ -147,13 +217,27 @@ sub create_rrd_graph {
     my $i = 0;
     foreach my $id (@ids) {
         my $col = $coldef[$i++] || die "fixme: no color definition";
-        push @args, "DEF:${id}=$rrd:${id}:$cf";
         my $dataid = $id;
+        my $linedef = "DEF:${dataid}=$rrd:${id}:$cf";
+        $linedef = "${linedef}:start=${last_old}" if $use_old; # avoid eventual overlap
+
+        push @args, "${linedef}";
+
         if ($id eq 'cpu' || $id eq 'iowait') {
-            push @args, "CDEF:${id}_per=${id},100,*";
+            push @args, "CDEF:${dataid}_per=${id},100,*";
             $dataid = "${id}_per";
         }
         push @args, "LINE2:${dataid}${col}:${id}";
+
+        if ($use_old) {
+            my $dataid = "${id}old";
+            push @args, "DEF:${dataid}=$old_rrd:${id}:${cf}";
+            if ($id eq 'cpu' || $id eq 'iowait') {
+                push @args, "CDEF:${dataid}_per=${dataid},100,*";
+                $dataid = "${dataid}_per";
+            }
+            push @args, "LINE2:${dataid}${col}";
+        }
     }
 
     push @args, '--full-size-mode';
-- 
2.47.2



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


  parent reply	other threads:[~2025-09-05 13:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-05 13:55 [pve-devel] [PATCH cluster/container/manager/qemu-server/storage 0/7] combine and simplify RRD handling Aaron Lauterer
2025-09-05 13:55 ` [pve-devel] [PATCH cluster 1/3] rrd: fix rrd time frames Aaron Lauterer
2025-09-05 13:55 ` Aaron Lauterer [this message]
2025-09-05 13:55 ` [pve-devel] [PATCH cluster 3/3] pmxcfs: status.c: always use 9.0 rrd files Aaron Lauterer
2025-09-05 13:55 ` [pve-devel] [PATCH manager 1/1] status: rrddata: use fixed pve-node-9.0 path Aaron Lauterer
2025-09-05 13:55 ` [pve-devel] [PATCH qemu-server 1/1] status: rrddata: use fixed pve-vm-9.0 path Aaron Lauterer
2025-09-05 13:55 ` [pve-devel] [PATCH container " Aaron Lauterer
2025-09-05 13:55 ` [pve-devel] [PATCH storage 1/1] status: rrddata: use fixed pve-storage-9.0 path Aaron Lauterer
  -- strict thread matches above, loose matches on Subject: below --
2025-09-04 14:09 [pve-devel] [RFC many 0/3] combine and simplify RRD handling Aaron Lauterer
2025-09-04 14:09 ` [pve-devel] [PATCH cluster 2/3] RRD: fetch data from old rrd file if present and needed Aaron Lauterer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250905135517.4005478-3-a.lauterer@proxmox.com \
    --to=a.lauterer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal