[pve-devel] [PATCH manager 1/4] API2Tools: rrd: remove O(n^2) lookup for keys
Aaron Lauterer
a.lauterer at proxmox.com
Fri Sep 5 15:58:18 CEST 2025
On 2025-09-05 15:52, Dominik Csapak wrote:
>
>
> On 9/5/25 3:49 PM, Aaron Lauterer wrote:
>>
>>
>> On 2025-09-05 14:06, Dominik Csapak wrote:
>>> the idea was that we get any of the 'new' versions on lookup, but that
>>> lead to iterating through possibly all keys. Since that was called for
>>> each resource in e.g. /cluster/resources api call, the runtime was
>>> O(n^2) for the number of resources.
>>>
>>> To avoid that, simply look up the currently only valid key here which
>>> makes this lookup much cheaper.
>>>
>>> In my test setup with ~10000 guests, it reduces the time for a call
>>> to /cluster/resources from ~22s to ~400ms
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>>> ---
>>> NOTE: this is only a workaround and Aaron is working to making this
>>> whole part unnecessary, but for now i think this is a good stop-gap
>>>
>>> PVE/API2Tools.pm | 7 +++----
>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/PVE/API2Tools.pm b/PVE/API2Tools.pm
>>> index abe73fa2..863f5f55 100644
>>> --- a/PVE/API2Tools.pm
>>> +++ b/PVE/API2Tools.pm
>>> @@ -52,10 +52,9 @@ sub get_rrd_key {
>>> return "pve2.3-${type}/${id}";
>>> }
>>> - # if no old key has been found, we expect on in the newer
>>> format: pve-{type}-{version}/{id}
>>> - # We accept all new versions, as the expectation is that they
>>> are only allowed to add new colums as non-breaking change
>>> - for my $k (keys %$rrd) {
>>> - return $k if $k =~ m/^pve-\Q${type}\E-\d\d?.\d\/\Q${id}\E$/;
>>> + my $key = "pve-${type}-9.0/${id}";
>>> + if (defined($rrd->{$key})) {
>>> + return $key;
>>
>> Have you tested this in a mixed PVE8 + PVE9 cluster? This might break
>> it and we might have to keep the dynamic checking around.
>>
>> I don't have too much time right now. But I think this would break a
>> mixed version situation, e.g. during a cluster upgrade, where we
>> receive both keys, pve2-... and pve-...-9.0 ones.
>>
>>> }
>>> }
>>
>
> but isn't that why the checks above (outside the context) are there?:
you are right! sorry for the noise
>
> ---
> # check for old formats: pve2-{type}/{id}. For VMs and CTs the version
> number is different than for nodes and storages
> if ($type ne "vm" && exists $rrd->{"pve2-${type}/${id}"}) {
> return "pve2-${type}/${id}";
> } elsif ($type eq "vm" && exists $rrd->{"pve2.3-${type}/${id}"}) {
> return "pve2.3-${type}/${id}";
> }
> ---
>
> ?
More information about the pve-devel
mailing list