[pve-devel] [PATCH cluster-pve8 1/1] status: handle new metrics update data

[Aaron Lauterer <a.lauterer@proxmox.com>]

For PVE9 we plan to add additional fields in the metrics that are
collected and distributed in the cluster. The new fields/columns are
added at the end of the current ones. This makes it possible for PVE8
installations to still use them by cutting the new additional data.

To make it more future proof, the format of the keys for each metrics
are changed:

Old: pve{version}-{type}/{id}
New: pve-{type}-{version}/{id}

This way we have an easier time to handle new versions in the future as
we initially only need to check for `pve-{type}-`. If we know the
version, we can handle it accordingly; e.g. pad if older format with
missing data. If we don't know the version, it must be a newer one and
we cut the data stream at the length we need for the current version.

This means of course that to avoid a breaking change, we can only add
new columns if needed, but not remove any! But waiting for a breaking
change until the next major release is a worthy trade-off if it allows
us to expand the format in between if needed.

Since the full keys were used for the final location within the RRD
directory, we need to change that as well and set it manually to
'pve2-{type}' as the key we receive could be for a newer data format.

The 'rrd_skip_data' function got a new parameter defining the sepataring
character. This then makes it possible to use it to determine which part
of the key string is the version/type and which one is the actual
resource identifier.

We drop the pve2-vm schema as the newer pve2.3-vm has been introduced
with commit ba9dcfc1 back in 2013. By now there should be no cluster
where an older node might still send the old pve2-vm schema.

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

Notes:
    changes since:
    RFC:
    * rebased on stable/bookworm
    * formatted code with clang-format as in 717b6ea
    * switch from pve{version}-{type} to pve-{type}-{version} schema for new
      versions
    * expand rrd_skip_data with parameter for separating character
    * improve data cutoff according to recommendations, dropping the buffer
      for PVE8
    * renamed 'data_cutoff' to 'keep_columns' and push the skip to include
      the full last column centrally where we do the actual cutting
    * include dropping of pve2-vm handling which used to be a separate patch

 src/pmxcfs/status.c | 85 ++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 32 deletions(-)

diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index 0895e53..640540f 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -1185,16 +1185,33 @@ static void create_rrd_file(const char *filename, int argcount, const char *rrdd
     }
 }
 
-static inline const char *rrd_skip_data(const char *data, int count) {
+static inline const char *rrd_skip_data(const char *data, int count, char separator) {
     int found = 0;
     while (*data && found < count) {
-        if (*data++ == ':') {
+        if (*data++ == separator) {
             found++;
         }
     }
     return data;
 }
 
+// The key and subdirectory format used up until PVE8 is 'pve{version}-{type}/{id}' with version
+// being 2 or 2.3 for VMs. Starting with PVE9 'pve-{type}-{version}/{id}'. Newer versions are only
+// allowed to append new columns to the data! Otherwise this would be a breaking change.
+//
+// Type can be: node, vm, storage
+//
+// Version is the version of PVE with which it was introduced, e.g.: 9.0, 9.2, 10.0.
+//
+// ID is the actual identifier of the item in question. E.g. node name, VMID or for storage it is
+// '{node}/{storage name}'
+//
+// This way, we can handle unknown new formats gracefully and cut the data at the expected
+// column for the currently understood format. Receiving older formats will still need special
+// checks to determine how much padding is needed.
+//
+// Should we ever plan to change existing columns, we need to introduce this as a breaking
+// change!
 static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
     g_return_if_fail(key != NULL);
     g_return_if_fail(data != NULL);
@@ -1210,12 +1227,13 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
 
     char *filename = NULL;
 
-    int skip = 0;
-
-    if (strncmp(key, "pve2-node/", 10) == 0) {
-        const char *node = key + 10;
+    int skip = 0; // columns to skip at beginning. They contain non-archivable data, like uptime,
+                  // status, is guest a template and such.
+    int keep_columns = 0; // how many columns do we want to keep (after initial skip) in case we get
+                          // more columns than needed from a newer format
 
-        skip = 2;
+    if (strncmp(key, "pve2-node/", 10) == 0 || strncmp(key, "pve-node-", 9) == 0) {
+        const char *node = rrd_skip_data(key, 1, '/');
 
         if (strchr(node, '/') != NULL) {
             goto keyerror;
@@ -1225,25 +1243,23 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
             goto keyerror;
         }
 
-        filename = g_strdup_printf(RRDDIR "/%s", key);
+        skip = 2; // first two columns are live data that isn't archived
 
-        if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
+        if (strncmp(key, "pve-node-", 9) == 0) {
+            keep_columns = 12; // pve2-node format uses 12 columns
+        }
+
+        filename = g_strdup_printf(RRDDIR "/pve2-node/%s", node);
 
+        if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
             mkdir(RRDDIR "/pve2-node", 0755);
             int argcount = sizeof(rrd_def_node) / sizeof(void *) - 1;
             create_rrd_file(filename, argcount, rrd_def_node);
         }
 
-    } else if ((strncmp(key, "pve2-vm/", 8) == 0) || (strncmp(key, "pve2.3-vm/", 10) == 0)) {
-        const char *vmid;
+    } else if (strncmp(key, "pve2.3-vm/", 10) == 0 || strncmp(key, "pve-vm-", 7) == 0) {
 
-        if (strncmp(key, "pve2-vm/", 8) == 0) {
-            vmid = key + 8;
-            skip = 2;
-        } else {
-            vmid = key + 10;
-            skip = 4;
-        }
+        const char *vmid = rrd_skip_data(key, 1, '/');
 
         if (strchr(vmid, '/') != NULL) {
             goto keyerror;
@@ -1253,29 +1269,29 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
             goto keyerror;
         }
 
+        skip = 4; // first 4 columns are live data that isn't archived
+
+        if (strncmp(key, "pve-vm-", 7) == 0) {
+            keep_columns = 10; // pve2.3-vm format uses 10 data columns
+        }
+
         filename = g_strdup_printf(RRDDIR "/%s/%s", "pve2-vm", vmid);
 
         if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
-
             mkdir(RRDDIR "/pve2-vm", 0755);
             int argcount = sizeof(rrd_def_vm) / sizeof(void *) - 1;
             create_rrd_file(filename, argcount, rrd_def_vm);
         }
 
-    } else if (strncmp(key, "pve2-storage/", 13) == 0) {
-        const char *node = key + 13;
+    } 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}
 
-        const char *storage = node;
-        while (*storage && *storage != '/') {
-            storage++;
-        }
+        const char *storage = rrd_skip_data(node, 1, '/');
 
-        if (*storage != '/' || ((storage - node) < 1)) {
+        if ((storage - node) < 1) {
             goto keyerror;
         }
 
-        storage++;
-
         if (strchr(storage, '/') != NULL) {
             goto keyerror;
         }
@@ -1284,12 +1300,10 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
             goto keyerror;
         }
 
-        filename = g_strdup_printf(RRDDIR "/%s", key);
+        filename = g_strdup_printf(RRDDIR "/pve2-storage/%s", node);
 
         if (!g_file_test(filename, G_FILE_TEST_EXISTS)) {
-
             mkdir(RRDDIR "/pve2-storage", 0755);
-
             char *dir = g_path_get_dirname(filename);
             mkdir(dir, 0755);
             g_free(dir);
@@ -1302,7 +1316,14 @@ static void update_rrd_data(const char *key, gconstpointer data, size_t len) {
         goto keyerror;
     }
 
-    const char *dp = skip ? rrd_skip_data(data, skip) : data;
+    const char *dp = skip ? rrd_skip_data(data, skip, ':') : data;
+
+    if (keep_columns) {
+        keep_columns++; // We specify the number of columns we want earlier, but we also have the
+                        // always present timestamp column, so we need to skip one more column
+        char *cut = (char *)rrd_skip_data(dp, keep_columns, ':');
+        *(cut - 1) = 0; // terminate string by replacing colon from field separator with zero.
+    }
 
     const char *update_args[] = {dp, NULL};
 
-- 
2.39.5



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