Re: [pve-devel] [PATCH ha-manager v2 8/8] implement static service stats cache

[Fiona Ebner <f.ebner@proxmox.com>]

Am 23.10.25 um 6:19 PM schrieb Daniel Kral:
> On Mon Oct 20, 2025 at 6:45 PM CEST, Daniel Kral wrote:
>> @@ -502,6 +504,39 @@ sub get_datacenter_settings {
>>      };
>>  }
>>  
>> +sub get_static_service_stats {
>> +    my ($self, $id) = @_;
>> +
>> +    # undef if update_static_service_stats(...) failed before
>> +    return undef if !defined($self->{static_service_stats});
>> +
>> +    return $self->{static_service_stats}->{$id};
>> +}
>> +
>> +sub update_static_service_stats {
>> +    my ($self) = @_;
>> +
>> +    my $properties = ['cores', 'cpulimit', 'memory', 'sockets', 'vcpus'];
>> +    my $service_stats = eval {
>> +        my $stats = PVE::Cluster::get_guest_config_properties($properties);
>> +
>> +        # get_guest_config_properties(...) doesn't add guests which do not
>> +        # specify any of the given properties, but we need to make a distinction
>> +        # between "not cached" and "not specified" here
>> +        my $vmlist = PVE::Cluster::get_vmlist();
>> +        for my $id (keys %$vmlist) {
> 
> I'm sorry, this is obviously wrong, it should be
> 
>     my $vmlist = PVE::Cluster::get_vmlist() // {};
>     my $idlist = $vmlist->{ids} // {};
>     for my $id (keys %$idlist) {
> 
> here. This will only result in performance degradation as all guests
> without any of the given guest config $properties specified will then
> fallback to reading their guest config directly.

Good catch! As we discussed off-list, the "// {}" for get_vmlist() is
unnecessary and since we do a cfs_update() in each iteration, the list
should be up-to-date too.

> If nothing else comes up until Tuesday I'll send a v3 or a follow up.
> 
> @Fiona, can I still apply your R-b then?

Yes.


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