Re: [pdm-devel] [PATCH proxmox v4 06/20] rrd: try to load database if not already present in cache

Re: [pdm-devel] [PATCH proxmox v4 06/20] rrd: try to load database if not already present in cache

[Lukas Wagner]

On  2024-09-19 11:30, Wolfgang Bumiller wrote:
> This actually needs to be opt-in (or opt-out).
> The API calls to query RRD data will now *always succeed* even if
> specifying a wrong node name for instance.
> 
>     $ proxmox-datacenter-manager-client pve node rrddata pve8a anonexistingnodethatwillneverexist MAX hour
>     []
> 

Finally got around to taking a look at this, sorry for the delay.

Fixing this is actually quite tricky.

Consider the following scenario:

 - add a new remote (with nodes A and B)
 - query rrddata for node 'A' *before* the first metric collection round

In this case, we probably want to return an empty response (no data yet).
However, if we

 - query rrddata for node 'C' (which does not exist)

we would like to return an error, because the node is not known. 
However, it is tricky to distinguish between these two
cases. Of course, we could retrieve a list of all nodes via the API and differentiate the cases
based on that, but even then we still can have a race condition where a new node was added
to the PVE cluster but we did fetch any metric data for it yet.
Also, we don't want do do an API request to the remote each and every time
we query RRD data. If we add caching for the list of known nodes, we increase
the risk for the aforementioned race condition.

TBH I'd just leave it as is, returning an empty reponse instead of an error
seems like a cosmetical issue to me. The number of users using the this API
for custom metric visualization is probably very low.

What do you think?

-- 
- Lukas



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