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

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

Am 16.10.25 um 5:15 PM schrieb Daniel Kral:
> On Thu Oct 16, 2025 at 1:12 PM CEST, Fiona Ebner wrote:
>> Am 30.09.25 um 4:21 PM schrieb Daniel Kral:
>>> @@ -497,6 +499,25 @@ 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} // {};
>>
>> Can't returning '{}' when nothing is there lead to issues down the line?
>> If we return undef instead, it's consistent with not having anything
>> cached and the caller will fall back to loading the config.
> 
> This return value type definitely needs improvement and/or better
> documentation, but an undef $self->{static_service_stats}->{$id} value
> indicates that it should fallback to the default value as none of the
> properties requested by get_guest_config_properties(...) was included in
> that particular guest config, e.g. no 'cores', 'sockets', and 'memory'
> properties defined in a VM config.
> When $self->{static_service_stats} itself is undef, then the static
> cache couldn't be queried for some reason.

Okay, so get_guest_config_properties() only includes guests that do have
one of the queried properties explicitly set in its result. Thus, we
cannot distinguish between the cache being created at a time when a
guest did not exist yet or a guest with none of the queried properties
explicitly set. If returning {} as a fallback, we get the wrong values
in the former case, if returning undef as a fallback, we just have to
explicitly load the config in the latter case. There probably are not
many setups with many guests without any of the queried properties
explicitly set, so that is unlikely to hurt performance in practice.


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