[pve-devel] [PATCH cluster/container/manager/qemu-server/storage 0/7] combine and simplify RRD handling

[pve-devel] [PATCH cluster/container/manager/qemu-server/storage 0/7] combine and simplify RRD handling

[Aaron Lauterer]

this series does two things:
* switch the RRD API backend to use the old RRD files is available
  * only data for frontend graphs
  * PNG generation
* always use RRD files with the new 9.0 schema in pmxcfs/status.c
* hardcode pve-TYPE-9.0 paths in the RRD api endpoints for
  * nodes
  * VMs
  * CTs
  * Storage

The motivation for the combination of old and new RRD files is that during the
migration of old RRD files to the new ones, spikes get flattened. By combining
the old and new RRD files, we can keep the old coarse data with all it spikes
and only show the new much mode finer stepped data where available.

This also enables us to just write any new data into an RRD file that is
created with the new 9.0 schema. As a result, no migration step is needed
anymore and we can simplify the logic around the whole "does the only the old
file exist or is the new one also present" alot in quite a few places.

This series does not remove the rrd migration tool or its dependencies. That
still needs to happen. We want to keep the check in the pve8to9 tool, because we
will need the additional space in any case.

cluster:

Aaron Lauterer (3):
  rrd: fix rrd time frames
  RRD: fetch data from old rrd file if present and needed
  pmxcfs: status.c: always use 9.0 rrd files

 src/PVE/RRD.pm      | 162 ++++++++++++++++++++++--------
 src/pmxcfs/status.c | 238 ++++++--------------------------------------
 2 files changed, 153 insertions(+), 247 deletions(-)


manager:

Aaron Lauterer (1):
  status: rrddata: use fixed pve-node-9.0 path

 PVE/API2/Nodes.pm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)


qemu-server:

Aaron Lauterer (1):
  status: rrddata: use fixed pve-vm-9.0 path

 src/PVE/API2/Qemu.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)


container:

Aaron Lauterer (1):
  status: rrddata: use fixed pve-vm-9.0 path

 src/PVE/API2/LXC.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)


storage:

Aaron Lauterer (1):
  status: rrddata: use fixed pve-storage-9.0 path

 src/PVE/API2/Storage/Status.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


Summary over all repositories:
  6 files changed, 176 insertions(+), 270 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pve-devel] [PATCH cluster 1/3] rrd: fix rrd time frames

[Aaron Lauterer]

month with the new rrd files provides a resoltion of 30 minutes

year had a typo 1140 which should be 1440

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

diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
index 8fe1927..932191d 100644
--- a/src/PVE/RRD.pm
+++ b/src/PVE/RRD.pm
@@ -28,8 +28,8 @@ sub create_rrd_data {
         hour => [60, 60], # 1 min resolution
         day => [60, 1440], # 1 min resolution, full day
         week => [60 * 30, 336], # 30 min resolution, 7 days
-        month => [3600 * 6, 121], # 6 hour resolution, 30 days, need one more count. Otherwise RRD gets wrong $step
-        year => [3600 * 6, 1140], # 6 hour resolution, 360 days
+        month => [60 * 30, 1440], # 30 min resolution  30 days
+        year => [3600 * 6, 1440], # 6 hour resolution, 360 days
         decade => [86400 * 7, 570], # 1 week resolution, 10 years
     };
 
@@ -115,8 +115,8 @@ sub create_rrd_graph {
         hour => [60, 60], # 1 min resolution
         day => [60, 1440], # 1 min resolution, full day
         week => [60 * 30, 336], # 30 min resolution, 7 days
-        month => [3600 * 6, 121], # 6 hour resolution, 30 days, need one more count. Otherwise RRD gets wrong $step
-        year => [3600 * 6, 1140], # 6 hour resolution, 360 days
+        month => [60 * 30, 1440], # 30 min resolution  30 days
+        year => [3600 * 6, 1440], # 6 hour resolution, 360 days
         decade => [86400 * 7, 570], # 1 week resolution, 10 years
     };
 
-- 
2.47.2



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

[pve-devel] [PATCH cluster 2/3] RRD: fetch data from old rrd file if present and needed

[Aaron Lauterer]

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

[pve-devel] [PATCH cluster 3/3] pmxcfs: status.c: always use 9.0 rrd files

[Aaron Lauterer]

Assuming that we can keep the old RRD files around and fetch data
directly there if needed, we can skip the migration step and just write
to new 9.0 RRD files.

This simplifies the whole approach introduced in
9b00ac9 status: introduce new pve-{type}- rrd and metric format

We will always create a new 9.0 file if not present and then need to
handle the following cases:

* old pve2-{type} format -> pad missing columns
* pve-{type}-9.0 matches -> no changes needed
* pve-{type}- received, but version doesn't match -> new format, cut at
  expected columns

Since we don't need the old RRD definitions anymore, they are removed.
Otherwise the compile will throw an unused variable warning.
The `rrd_def_{type}_columns` variables get a `_pve2` suffix to make it
clear what they are for.

The `checked_mkdir` pattern for VMs is aligned with how we do it for
nodes and storages, to call it once more, to be safe.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

Notes:
    if I misunderstood the reason for the doulbe checked_mkdir, then it
    might only be needed for the storage due to the additional subdirs?

 src/pmxcfs/status.c | 238 ++++++--------------------------------------
 1 file changed, 30 insertions(+), 208 deletions(-)

diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index 130f08f..cb03e4e 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -1097,36 +1097,9 @@ kventry_hash_set(GHashTable *kvhash, const char *key, gconstpointer data, size_t
 // We create the RRD files with a 60 second stepsize, therefore, RRA timesteps
 // are alwys per 60 seconds. These 60 seconds are usually showing up in other
 // code paths where we interact with RRD data!
-static const char *rrd_def_node[] = {
-    "DS:loadavg:GAUGE:120:0:U",
-    "DS:maxcpu:GAUGE:120:0:U",
-    "DS:cpu:GAUGE:120:0:U",
-    "DS:iowait:GAUGE:120:0:U",
-    "DS:memtotal:GAUGE:120:0:U",
-    "DS:memused:GAUGE:120:0:U",
-    "DS:swaptotal:GAUGE:120:0:U",
-    "DS:swapused:GAUGE:120:0:U",
-    "DS:roottotal:GAUGE:120:0:U",
-    "DS:rootused:GAUGE:120:0:U",
-    "DS:netin:DERIVE:120:0:U",
-    "DS:netout:DERIVE:120:0:U",
-
-    "RRA:AVERAGE:0.5:1:70",     // 1 min avg - one hour
-    "RRA:AVERAGE:0.5:30:70",    // 30 min avg - one day
-    "RRA:AVERAGE:0.5:180:70",   // 3 hour avg - one week
-    "RRA:AVERAGE:0.5:720:70",   // 12 hour avg - one month
-    "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year
-
-    "RRA:MAX:0.5:1:70",     // 1 min max - one hour
-    "RRA:MAX:0.5:30:70",    // 30 min max - one day
-    "RRA:MAX:0.5:180:70",   // 3 hour max - one week
-    "RRA:MAX:0.5:720:70",   // 12 hour max - one month
-    "RRA:MAX:0.5:10080:70", // 7 day max - ony year
-    NULL,
-};
 
 // This *must* be the number of columns as defined above.
-static const int rrd_def_node_columns = 12;
+static const int rrd_def_node_columns_pve2 = 12;
 
 static const char *rrd_def_node_pve9_0[] = {
     "DS:loadavg:GAUGE:120:0:U",
@@ -1165,34 +1138,8 @@ static const char *rrd_def_node_pve9_0[] = {
 // This *must* be the number of columns as defined above.
 static const int rrd_def_node_pve9_0_columns = 19;
 
-static const char *rrd_def_vm[] = {
-    "DS:maxcpu:GAUGE:120:0:U",
-    "DS:cpu:GAUGE:120:0:U",
-    "DS:maxmem:GAUGE:120:0:U",
-    "DS:mem:GAUGE:120:0:U",
-    "DS:maxdisk:GAUGE:120:0:U",
-    "DS:disk:GAUGE:120:0:U",
-    "DS:netin:DERIVE:120:0:U",
-    "DS:netout:DERIVE:120:0:U",
-    "DS:diskread:DERIVE:120:0:U",
-    "DS:diskwrite:DERIVE:120:0:U",
-
-    "RRA:AVERAGE:0.5:1:70",     // 1 min avg - one hour
-    "RRA:AVERAGE:0.5:30:70",    // 30 min avg - one day
-    "RRA:AVERAGE:0.5:180:70",   // 3 hour avg - one week
-    "RRA:AVERAGE:0.5:720:70",   // 12 hour avg - one month
-    "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year
-
-    "RRA:MAX:0.5:1:70",     // 1 min max - one hour
-    "RRA:MAX:0.5:30:70",    // 30 min max - one day
-    "RRA:MAX:0.5:180:70",   // 3 hour max - one week
-    "RRA:MAX:0.5:720:70",   // 12 hour max - one month
-    "RRA:MAX:0.5:10080:70", // 7 day max - ony year
-    NULL,
-};
-
 // This *must* be the number of columns as defined above.
-static const int rrd_def_vm_columns = 10;
+static const int rrd_def_vm_columns_pve2 = 10;
 
 static const char *rrd_def_vm_pve9_0[] = {
     "DS:maxcpu:GAUGE:120:0:U",
@@ -1229,24 +1176,6 @@ static const char *rrd_def_vm_pve9_0[] = {
 // This *must* be the number of columns as defined above.
 static const int rrd_def_vm_pve9_0_columns = 17;
 
-static const char *rrd_def_storage[] = {
-    "DS:total:GAUGE:120:0:U",
-    "DS:used:GAUGE:120:0:U",
-
-    "RRA:AVERAGE:0.5:1:70",     // 1 min avg - one hour
-    "RRA:AVERAGE:0.5:30:70",    // 30 min avg - one day
-    "RRA:AVERAGE:0.5:180:70",   // 3 hour avg - one week
-    "RRA:AVERAGE:0.5:720:70",   // 12 hour avg - one month
-    "RRA:AVERAGE:0.5:10080:70", // 7 day avg - ony year
-
-    "RRA:MAX:0.5:1:70",     // 1 min max - one hour
-    "RRA:MAX:0.5:30:70",    // 30 min max - one day
-    "RRA:MAX:0.5:180:70",   // 3 hour max - one week
-    "RRA:MAX:0.5:720:70",   // 12 hour max - one month
-    "RRA:MAX:0.5:10080:70", // 7 day max - ony year
-    NULL,
-};
-
 static const char *rrd_def_storage_pve9_0[] = {
     "DS:total:GAUGE:120:0:U",
     "DS:used:GAUGE:120:0:U",
@@ -1356,68 +1285,27 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
         }
 
         filename = g_strdup_printf(RRDDIR "/pve-node-9.0/%s", node);
-        char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-node/%s", node);
-
-        int use_pve2_file = 0;
 
-        // check existing rrd files and directories
-        if (g_file_test(filename, G_FILE_TEST_EXISTS)) {
-            // pve-node-9.0 file exists, we use that
-            // TODO: get conditions so, that we do not have this empty branch
-        } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) {
-            // old file exists, use it
-            use_pve2_file = 1;
-
-            g_free(filename);
-            filename = filename_pve2;
-            filename_pve2 = NULL;
-        } else {
-            // neither file exists, check for directories to decide and create file
+        if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
+            checked_mkdir(RRDDIR "/pve-node-9.0", 0755);
 
-            if (g_file_test(RRDDIR "/pve-node-9.0", G_FILE_TEST_IS_DIR)) {
+            char *dir = g_path_get_dirname(filename);
+            checked_mkdir(dir, 0755);
+            g_free(dir);
 
-                int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_node_pve9_0);
-            } else if (g_file_test(RRDDIR "/pve2-node", G_FILE_TEST_IS_DIR)) {
-                use_pve2_file = 1;
-
-                g_free(filename);
-                filename = filename_pve2;
-                filename_pve2 = NULL;
-
-                char *dir = g_path_get_dirname(filename);
-                checked_mkdir(dir, 0755);
-                g_free(dir);
-
-                int argcount = sizeof(rrd_def_node) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_node);
-            } else {
-                // no dir exists yet, use new pve-node-9.0
-                checked_mkdir(RRDDIR "/pve-node-9.0", 0755);
-
-                char *dir = g_path_get_dirname(filename);
-                checked_mkdir(dir, 0755);
-                g_free(dir);
-
-                int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_node_pve9_0);
-            }
+            int argcount = sizeof(rrd_def_node_pve9_0) / sizeof(void *) - 1;
+            create_rrd_file(filename, argcount, rrd_def_node_pve9_0);
         }
 
         skip = 2; // first two columns are live data that isn't archived
 
-        if (strncmp(key, "pve2-node/", 10) == 0 && !use_pve2_file) {
-            padding = rrd_def_node_pve9_0_columns - rrd_def_node_columns;
-        } else if (strncmp(key, "pve-node-", 9) == 0 && use_pve2_file) {
-            keep_columns = rrd_def_node_columns;
+        if (strncmp(key, "pve2-node/", 10) == 0) {
+            padding = rrd_def_node_pve9_0_columns - rrd_def_node_columns_pve2;
         } else if (strncmp(key, "pve-node-9.0/", 13) != 0) {
             // we received an unknown format, expectation is it is newer and has more columns
             // than we can currently handle
             keep_columns = rrd_def_node_pve9_0_columns;
         }
-
-        g_free(filename_pve2);
-
     } else if (strncmp(key, "pve2.3-vm/", 10) == 0 || strncmp(key, "pve-vm-", 7) == 0) {
 
         const char *vmid = rrd_skip_data(key, 1, '/');
@@ -1431,59 +1319,28 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
         }
 
         filename = g_strdup_printf(RRDDIR "/pve-vm-9.0/%s", vmid);
-        char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-vm/%s", vmid);
-
-        int use_pve2_file = 0;
-
-        // check existing rrd files and directories
-        if (g_file_test(filename, G_FILE_TEST_EXISTS)) {
-            // pve-vm-9.0 file exists, we use that
-            // TODO: get conditions so, that we do not have this empty branch
-        } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) {
-            // old file exists, use it
-            use_pve2_file = 1;
-            g_free(filename);
-            filename = filename_pve2;
-            filename_pve2 = NULL;
-        } else {
-            // neither file exists, check for directories to decide and create file
-
-            if (g_file_test(RRDDIR "/pve-vm-9.0", G_FILE_TEST_IS_DIR)) {
 
-                int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_vm_pve9_0);
-            } else if (g_file_test(RRDDIR "/pve2-vm", G_FILE_TEST_IS_DIR)) {
-                use_pve2_file = 1;
+        if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
+            // no dir exists yet, use new pve-vm-9.0
+            checked_mkdir(RRDDIR "/pve-vm-9.0", 0755);
 
-                g_free(filename);
-                filename = filename_pve2;
-                filename_pve2 = NULL;
+            char *dir = g_path_get_dirname(filename);
+            checked_mkdir(dir, 0755);
+            g_free(dir);
 
-                int argcount = sizeof(rrd_def_vm) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_vm);
-            } else {
-                // no dir exists yet, use new pve-vm-9.0
-                checked_mkdir(RRDDIR "/pve-vm-9.0", 0755);
-
-                int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_vm_pve9_0);
-            }
+            int argcount = sizeof(rrd_def_vm_pve9_0) / sizeof(void *) - 1;
+            create_rrd_file(filename, argcount, rrd_def_vm_pve9_0);
         }
 
         skip = 4; // first 4 columns are live data that isn't archived
 
-        if (strncmp(key, "pve2.3-vm/", 10) == 0 && !use_pve2_file) {
-            padding = rrd_def_vm_pve9_0_columns - rrd_def_vm_columns;
-        } else if (strncmp(key, "pve-vm-", 7) == 0 && use_pve2_file) {
-            keep_columns = rrd_def_vm_columns;
+        if (strncmp(key, "pve2.3-vm/", 10) == 0) {
+            padding = rrd_def_vm_pve9_0_columns - rrd_def_vm_columns_pve2;
         } else if (strncmp(key, "pve-vm-9.0/", 11) != 0) {
             // we received an unknown format, expectation is it is newer and has more columns
             // than we can currently handle
             keep_columns = rrd_def_vm_pve9_0_columns;
         }
-
-        g_free(filename_pve2);
-
     } else if (strncmp(key, "pve2-storage/", 13) == 0 || strncmp(key, "pve-storage-", 12) == 0) {
         const char *node = rrd_skip_data(key, 1, '/'); // will contain {node}/{storage}
 
@@ -1502,49 +1359,17 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
         }
 
         filename = g_strdup_printf(RRDDIR "/pve-storage-9.0/%s", node);
-        char *filename_pve2 = g_strdup_printf(RRDDIR "/pve2-storage/%s", node);
-
-        // check existing rrd files and directories
-        if (g_file_test(filename, G_FILE_TEST_EXISTS)) {
-            // pve-storage-9.0 file exists, we use that
-            // TODO: get conditions so, that we do not have this empty branch
-        } else if (g_file_test(filename_pve2, G_FILE_TEST_EXISTS)) {
-            // old file exists, use it
-            g_free(filename);
-            filename = filename_pve2;
-            filename_pve2 = NULL;
-        } else {
-            // neither file exists, check for directories to decide and create file
-
-            if (g_file_test(RRDDIR "/pve-storage-9.0", G_FILE_TEST_IS_DIR)) {
-                char *dir = g_path_get_dirname(filename);
-                checked_mkdir(dir, 0755);
-                g_free(dir);
-
-                int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_storage_pve9_0);
-            } else if (g_file_test(RRDDIR "/pve2-storage", G_FILE_TEST_IS_DIR)) {
-                g_free(filename);
-                filename = filename_pve2;
-                filename_pve2 = NULL;
-
-                char *dir = g_path_get_dirname(filename);
-                checked_mkdir(dir, 0755);
-                g_free(dir);
-
-                int argcount = sizeof(rrd_def_storage) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_storage);
-            } else {
-                // no dir exists yet, use new pve-storage-9.0
-                checked_mkdir(RRDDIR "/pve-storage-9.0", 0755);
 
-                char *dir = g_path_get_dirname(filename);
-                checked_mkdir(dir, 0755);
-                g_free(dir);
+        if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
+            // no dir exists yet, use new pve-storage-9.0
+            checked_mkdir(RRDDIR "/pve-storage-9.0", 0755);
 
-                int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1;
-                create_rrd_file(filename, argcount, rrd_def_storage_pve9_0);
-            }
+            char *dir = g_path_get_dirname(filename);
+            checked_mkdir(dir, 0755);
+            g_free(dir);
+
+            int argcount = sizeof(rrd_def_storage_pve9_0) / sizeof(void *) - 1;
+            create_rrd_file(filename, argcount, rrd_def_storage_pve9_0);
         }
 
         // actual data columns didn't change between pve2-storage and pve-storage-9.0
@@ -1553,9 +1378,6 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
             // than we can currently handle
             keep_columns = rrd_def_storage_pve9_0_columns;
         }
-
-        g_free(filename_pve2);
-
     } else {
         goto keyerror;
     }
-- 
2.47.2



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

[pve-devel] [PATCH manager 1/1] status: rrddata: use fixed pve-node-9.0 path

[Aaron Lauterer]

Because we now always create it, should it not exists and old data is
used from the old files.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Nodes.pm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index f4f8b37d..303c1852 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -865,10 +865,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-node-9.0/$param->{node}";
-        $path = "pve2-node/$param->{node}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_graph($path, $param->{timeframe},
-            $param->{ds}, $param->{cf});
+        return PVE::RRD::create_rrd_graph(
+            "pve-node-9.0/$param->{node}", $param->{timeframe}, $param->{ds}, $param->{cf},
+        );
 
     },
 });
@@ -909,9 +908,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-node-9.0/$param->{node}";
-        $path = "pve2-node/$param->{node}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_data($path, $param->{timeframe}, $param->{cf});
+        return PVE::RRD::create_rrd_data(
+            "pve-node-9.0/$param->{node}", $param->{timeframe}, $param->{cf},
+        );
     },
 });
 
-- 
2.47.2



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

[pve-devel] [PATCH qemu-server 1/1] status: rrddata: use fixed pve-vm-9.0 path

[Aaron Lauterer]

Because we now always create it, should it not exists and old data is
used from the old files.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 2acacc22..69b533f2 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -1630,9 +1630,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-vm-9.0/$param->{vmid}";
-        $path = "pve2-vm/$param->{vmid}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_graph($path, $param->{timeframe}, $param->{ds}, $param->{cf});
+        return PVE::RRD::create_rrd_graph(
+            "pve-vm-9.0/$param->{vmid}", $param->{timeframe}, $param->{ds}, $param->{cf},
+        );
 
     },
 });
@@ -1674,9 +1674,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-vm-9.0/$param->{vmid}";
-        $path = "pve2-vm/$param->{vmid}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_data($path, $param->{timeframe}, $param->{cf});
+        return PVE::RRD::create_rrd_data(
+            "pve-vm-9.0/$param->{vmid}", $param->{timeframe}, $param->{cf},
+        );
     },
 });
 
-- 
2.47.2



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

[pve-devel] [PATCH container 1/1] status: rrddata: use fixed pve-vm-9.0 path

[Aaron Lauterer]

Because we now always create it, should it not exists and old data is
used from the old files.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/PVE/API2/LXC.pm | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 1f89c87..0abe705 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -718,9 +718,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-vm-9.0/$param->{vmid}";
-        $path = "pve2-vm/$param->{vmid}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_graph($path, $param->{timeframe}, $param->{ds}, $param->{cf});
+        return PVE::RRD::create_rrd_graph(
+            "pve-vm-9.0/$param->{vmid}", $param->{timeframe}, $param->{ds}, $param->{cf},
+        );
 
     },
 });
@@ -762,9 +762,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-vm-9.0/$param->{vmid}";
-        $path = "pve2-vm/$param->{vmid}" if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_data($path, $param->{timeframe}, $param->{cf});
+        return PVE::RRD::create_rrd_data(
+            "pve-vm-9.0/$param->{vmid}", $param->{timeframe}, $param->{cf},
+        );
     },
 });
 
-- 
2.47.2



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

[pve-devel] [PATCH storage 1/1] status: rrddata: use fixed pve-storage-9.0 path

[Aaron Lauterer]

Because we now always create it, should it not exists and old data is
used from the old files.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index ad8c753..94fed9b 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -415,10 +415,11 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $path = "pve-storage-9.0/$param->{node}/$param->{storage}";
-        $path = "pve2-storage/$param->{node}/$param->{storage}"
-            if !-e "/var/lib/rrdcached/db/${path}";
-        return PVE::RRD::create_rrd_data($path, $param->{timeframe}, $param->{cf});
+        return PVE::RRD::create_rrd_data(
+            "pve-storage-9.0/$param->{node}/$param->{storage}",
+            $param->{timeframe},
+            $param->{cf},
+        );
     },
 });
 
-- 
2.47.2



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

[pve-devel] [RFC many 0/3] combine and simplify RRD handling

[Aaron Lauterer]

this series does two things:
* switch the RRD API backend to use the old RRD files is available
* always use RRD files with the new 9.0 schema

The motivation for the combination of old and new RRD files is that during the
migration of old RRD files to the new ones, spikes get flattened. By combining
the old and new RRD files, we can keep the old coarse data with all it spikes
and only show the new much mode finer stepped data where available.

This will also enable us to just write any new data into an RRD file that is
created with the new 9.0 schema. As a result, no migration step is needed
anymore and we can simplify the logic around the whole "does the only the old
file exist or is the new one also present" alot in quite a few places.

Some examples which we can simplify or revert:
pve-manager: API2Tools::get_rrd_key can most likely be dropped
qemu-server, pve-storage, pve-container,…: rrd API endpoints which check which files exist
drop the whole migration step on upgrade

other TODOs:

RRD::create_rrd_graph -> see if we can combine both files into one graph. But we
are relying on RRD itself for this. So that might be interesting. On the other
hand, do we want to keep that API endpoint around? People are using it though.
So we need to weigh that decision accoringly.


This is currently marked as RFC. Tested by installing it on an existing 9.0.6
cluster. Added a 8.4 node with latest updates. Renamed {rrd}.old to {rrd}.

cluster:

Aaron Lauterer (3):
  rrd: fix rrd time frames
  RRD: fetch data from old rrd file if present and needed
  pmxcfs: status.c: always use 9.0 rrd files

 src/PVE/RRD.pm      | 113 ++++++++++++++-------
 src/pmxcfs/status.c | 238 ++++++--------------------------------------
 2 files changed, 108 insertions(+), 243 deletions(-)


Summary over all repositories:
  2 files changed, 108 insertions(+), 243 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pve-devel] [PATCH cluster 1/3] rrd: fix rrd time frames

[Aaron Lauterer]

month with the new rrd files provides a resoltion of 30 minutes

year had a typo 1140 which should be 1440

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

diff --git a/src/PVE/RRD.pm b/src/PVE/RRD.pm
index 8fe1927..34dafef 100644
--- a/src/PVE/RRD.pm
+++ b/src/PVE/RRD.pm
@@ -28,8 +28,8 @@ sub create_rrd_data {
         hour => [60, 60], # 1 min resolution
         day => [60, 1440], # 1 min resolution, full day
         week => [60 * 30, 336], # 30 min resolution, 7 days
-        month => [3600 * 6, 121], # 6 hour resolution, 30 days, need one more count. Otherwise RRD gets wrong $step
-        year => [3600 * 6, 1140], # 6 hour resolution, 360 days
+        month => [60 * 30, 1440], # 30 min resolution  30 days
+        year => [3600 * 6, 1440], # 6 hour resolution, 360 days
         decade => [86400 * 7, 570], # 1 week resolution, 10 years
     };
 
-- 
2.47.2



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