[pve-devel] [PATCH v2 pve-storage 7/7] stats: api: cache storage plugin status

[Lukas Wagner <l.wagner@proxmox.com>]

Cache storage plugin status so that pvestatd and API calls can use the
cached results, without having to query all storage plugins again.

Introduces the `ignore-cache` on some storage status API calls. By
default it is 0, but when set to 1 the values from the cache will be
ignored.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v1 -> v2:
    - Add `ignore-cache` paramter
    - use `get_or_update` method from Proxmox::RS::SharedCached
    - invalidated the cache in other cases (e.g. when allocating volumes,
      as this might change the amount of free space)

 src/PVE/API2/Storage/Config.pm  | 18 +++++++
 src/PVE/API2/Storage/Content.pm | 15 ++++++
 src/PVE/API2/Storage/Status.pm  | 38 ++++++++++++++-
 src/PVE/Storage.pm              | 84 ++++++++++++++++++++++++---------
 4 files changed, 131 insertions(+), 24 deletions(-)

diff --git a/src/PVE/API2/Storage/Config.pm b/src/PVE/API2/Storage/Config.pm
index e04b6ab..e0ee0d8 100755
--- a/src/PVE/API2/Storage/Config.pm
+++ b/src/PVE/API2/Storage/Config.pm
@@ -274,6 +274,12 @@ __PACKAGE__->register_method ({
 		die $err;
 	    }
 
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    warn "could not invalidate storage plugin status cache: $@\n" if $@;
+
 	    PVE::Storage::write_config($cfg);
 
 	}, "create storage failed");
@@ -373,6 +379,12 @@ __PACKAGE__->register_method ({
 		    ." in Proxmox VE 9. Use 'create-base-path' or 'create-subdirs' instead.\n"
 	    }
 
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    warn "could not invalidate storage plugin status cache: $@\n" if $@;
+
 	    PVE::Storage::write_config($cfg);
 
 	}, "update storage failed");
@@ -422,6 +434,12 @@ __PACKAGE__->register_method ({
 
 	    delete $cfg->{ids}->{$storeid};
 
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    warn "could not invalidate storage plugin status cache: $@\n" if $@;
+
 	    PVE::Storage::write_config($cfg);
 
 	}, "delete storage failed");
diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index fe0ad4a..cc4f5cd 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -224,6 +224,11 @@ __PACKAGE__->register_method ({
 					       $param->{format},
 					       $name, $size);
 
+	eval {
+	    # Invalidate cached plugin status on configuration changes.
+	    PVE::Storage::status_cache()->delete("storage_plugin_status");
+	};
+	warn "could not invalidate storage plugin status cache: $@\n" if $@;
 	return $volid;
     }});
 
@@ -459,6 +464,11 @@ __PACKAGE__->register_method ({
 		# Remove log file #318 and notes file #3972 if they still exist
 		PVE::Storage::archive_auxiliaries_remove($path);
 	    }
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    print "could not invalidate storage plugin status cache: $@\n" if $@;
 	};
 
 	my $id = (defined $ownervm ? "$ownervm@" : '') . $storeid;
@@ -549,6 +559,11 @@ __PACKAGE__->register_method ({
 	    # ssh to connect to local host (which is not needed
 	    my $sshinfo = PVE::SSHInfo::get_ssh_info($target_node);
 	    PVE::Storage::storage_migrate($cfg, $src_volid, $sshinfo, $target_sid, {'target_volname' => $target_volname});
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    print "could not invalidate storage plugin status cache: $@\n" if $@;
 
 	    print "DEBUG: end worker $upid\n";
 
diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index b2336e6..e048975 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -85,6 +85,12 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		default => 0,
 	    },
+	    'ignore-cache' => {
+		description => "Ignore cached storage plugin status",
+		type => 'boolean',
+		optional => 1,
+		default => 0,
+	    },
 	},
     },
     returns => {
@@ -158,7 +164,12 @@ __PACKAGE__->register_method ({
 
 	my $cfg = PVE::Storage::config();
 
-	my $info = PVE::Storage::storage_info($cfg, $param->{content}, $param->{format});
+	my $info = PVE::Storage::storage_info(
+	    $cfg,
+	    $param->{content},
+	    $param->{format},
+	    $param->{'ignore-cache'}
+	);
 
 	raise_param_exc({ storage => "No such storage." })
 	    if $param->{storage} && !defined($info->{$param->{storage}});
@@ -252,6 +263,12 @@ __PACKAGE__->register_method ({
 	properties => {
 	    node => get_standard_option('pve-node'),
 	    storage => get_standard_option('pve-storage-id'),
+	    'ignore-cache' => {
+		description => "Ignore cached storage plugin status",
+		type => 'boolean',
+		optional => 1,
+		default => 0,
+	    },
 	},
     },
     returns => {
@@ -263,7 +280,12 @@ __PACKAGE__->register_method ({
 
 	my $cfg = PVE::Storage::config();
 
-	my $info = PVE::Storage::storage_info($cfg, $param->{content});
+	my $info = PVE::Storage::storage_info(
+	    $cfg,
+	    $param->{content},
+	    undef,
+	    $param->{'ignore-cache'}
+	);
 
 	my $data = $info->{$param->{storage}};
 
@@ -528,6 +550,12 @@ __PACKAGE__->register_method ({
 	    unlink $tmpfilename; # the temporary file got only uploaded locally, no need to rm remote
 	    warn "unable to clean up temporary file '$tmpfilename' - $!\n" if $! && $! != ENOENT;
 
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    print "could not invalidate storage plugin status cache: $@\n" if $@;
+
 	    if (my $err = $@) {
 		eval { $err_cleanup->() };
 		warn "$@" if $@;
@@ -663,6 +691,12 @@ __PACKAGE__->register_method({
 		$opts->{decompression_command} = $info->{decompressor};
 	    }
 	    PVE::Tools::download_file_from_url("$path/$filename", $url, $opts);
+
+	    eval {
+		# Invalidate cached plugin status on configuration changes.
+		PVE::Storage::status_cache()->delete("storage_plugin_status");
+	    };
+	    print "could not invalidate storage plugin status cache: $@\n" if $@;
 	};
 
 	my $worker_id = PVE::Tools::encode_text($filename); # must not pass : or the like as w-ID
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 8ad493f..0c5c38f 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -23,6 +23,7 @@ use PVE::INotify;
 use PVE::RPCEnvironment;
 use PVE::SSHInfo;
 use PVE::RESTEnvironment qw(log_warn);
+use Proxmox::RS::SharedCache;
 
 use PVE::Storage::Plugin;
 use PVE::Storage::DirPlugin;
@@ -115,8 +116,13 @@ our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
 # FIXME remove with PVE 8.0, add versioned breaks for pve-manager
 our $vztmpl_extension_re = $VZTMPL_EXT_RE_1;
 
+
 #  PVE::Storage utility functions
 
+sub status_cache {
+    return Proxmox::RS::SharedCache->new("/run/pvestatd-cache");
+}
+
 sub config {
     return cfs_read_file("storage.cfg");
 }
@@ -1248,7 +1254,7 @@ sub deactivate_volumes {
 }
 
 sub storage_info {
-    my ($cfg, $content, $includeformat) = @_;
+    my ($cfg, $content, $includeformat, $ignore_cache) = @_;
 
     my $ids = $cfg->{ids};
 
@@ -1287,35 +1293,69 @@ sub storage_info {
 	push @$slist, $storeid;
     }
 
-    my $cache = {};
 
-    foreach my $storeid (keys %$ids) {
-	my $scfg = $ids->{$storeid};
+    my $get_plugin_status = sub {
+	my $cache = {};
+	my $status = {};
 
-	next if !$info->{$storeid};
-	next if !$info->{$storeid}->{enabled};
+	foreach my $storeid (keys %$ids) {
+	    my $scfg = $ids->{$storeid};
 
-	my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
-	if ($includeformat) {
-	    my $pd = $plugin->plugindata();
-	    $info->{$storeid}->{format} = $pd->{format}
-		if $pd->{format};
-	    $info->{$storeid}->{select_existing} = $pd->{select_existing}
-		if $pd->{select_existing};
+	    next if !$info->{$storeid};
+	    next if !$info->{$storeid}->{enabled};
+
+	    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+	    if ($includeformat) {
+		my $pd = $plugin->plugindata();
+		$info->{$storeid}->{format} = $pd->{format}
+		    if $pd->{format};
+		$info->{$storeid}->{select_existing} = $pd->{select_existing}
+		    if $pd->{select_existing};
+	    }
+
+	    eval { activate_storage($cfg, $storeid, $cache); };
+	    if (my $err = $@) {
+		warn $err;
+		next;
+	    }
+	    $status->{$storeid} = eval { $plugin->status($storeid, $scfg, $cache); };
+
+	    my ($total, $avail, $used, $active) = eval { $plugin->status($storeid, $scfg, $cache); };
+	    warn $@ if $@;
+	    $status->{$storeid} = {};
+
+	    $status->{$storeid}->{total} = int($total);
+	    $status->{$storeid}->{avail} = int($avail);
+	    $status->{$storeid}->{used} = int($used);
+	    $status->{$storeid}->{active} = $active;
 	}
 
-	eval { activate_storage($cfg, $storeid, $cache); };
-	if (my $err = $@) {
-	    warn $err;
-	    next;
+	return $status;
+    };
+
+    my $status;
+    if ($ignore_cache) {
+	$status = $get_plugin_status->();
+    } else {
+	eval {
+	    $status = status_cache()->get_or_update("storage_plugin_status", $get_plugin_status, 60)
+	};
+	if ($@) {
+	    warn "could not fetch cached storage plugin status: $@";
+	    $status = $get_plugin_status->();
 	}
+    }
 
-	my ($total, $avail, $used, $active) = eval { $plugin->status($storeid, $scfg, $cache); };
-	warn $@ if $@;
+
+    foreach my $storeid (keys %$ids) {
+	next if !$info->{$storeid};
+	next if !$info->{$storeid}->{enabled};
+	my $active = $status->{$storeid}->{active};
 	next if !$active;
-	$info->{$storeid}->{total} = int($total);
-	$info->{$storeid}->{avail} = int($avail);
-	$info->{$storeid}->{used} = int($used);
+
+	$info->{$storeid}->{total} = $status->{$storeid}->{total};
+	$info->{$storeid}->{avail} = $status->{$storeid}->{avail};
+	$info->{$storeid}->{used} = $status->{$storeid}->{used};
 	$info->{$storeid}->{active} = $active;
     }
 
-- 
2.39.2