From: Lukas Wagner <l.wagner@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [pdm-devel] [PATCH proxmox v4 06/20] rrd: try to load database if not already present in cache
Date: Thu, 2 Jan 2025 11:47:51 +0100 [thread overview]
Message-ID: <10a36f74-e682-4495-bf8a-91ddd3b8e59c@proxmox.com> (raw)
In-Reply-To: <zzimmmhhxl63oxh7rhgio3h4vfdvgxhhf3my6uh7vclgxalzpu@isagiomxldtv>
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
parent reply other threads:[~2025-01-02 10:48 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <zzimmmhhxl63oxh7rhgio3h4vfdvgxhhf3my6uh7vclgxalzpu@isagiomxldtv>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10a36f74-e682-4495-bf8a-91ddd3b8e59c@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=pdm-devel@lists.proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox