public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: [pdm-devel] superseded: [PATCH proxmox-datacenter-manager v2 00/28] metric collection improvements (concurrency, config, API, CLI)
Date: Wed, 16 Apr 2025 15:03:53 +0200	[thread overview]
Message-ID: <a5c2f0dc-1090-4f4f-aa0e-895b1f877a18@proxmox.com> (raw)
In-Reply-To: <88c03c89-f8e1-4538-94ad-89b829a6c06c@proxmox.com>

Sorry for the late reply.

On  2025-03-16 22:45, Thomas Lamprecht wrote:
> On 14/03/2025 15:10, Lukas Wagner wrote:
>> On  2025-02-14 14:06, Lukas Wagner wrote:
>>> ## To reviewers / open questions:
>>> - Please review the defaults I've chosen for the settings, especially
>>>   the ones for the default metric collection interval (10 minutes) as
>>>   well as max-concurrency (10).
>>>   I also kindly ask to double-check the naming of the properties.
>>>   See "pdm-api-types: add CollectionSettings type" for details
>>>
>>> - Please review path and params for new API endpoints (anything public
>>>   facing that is hard to change later)
>>>
>>> - I've chosen a section-config config now, even though we only have a
>>>   single section for now. This was done for future-proofing reasons,
>>>   maybe we want to add support for different setting 'groups' or
>>>   something, e.g. to have different settings for distinct sets of
>>>   remotes. Does this make sense?
>>>   Or should I just stick to a simple config for now? (At moments like
>>>   these I wish for TOML configs where we could be a bit more flexible...)
>>>
>>> 	collection-settings: default
>>> 	    max-concurrency 10
>>> 	    collection-interval 180
>>> 	    min-interval-offset 0
>>> 	    max-interval-offset 20
>>> 	    min-connection-delay 10
>>> 	    max-connection-delay 100
>>>
>>
>> Currently thinking about generalizing the `max-concurrency` setting into something global
>> that affects all 'background' polling operations (resource cache/task cache refreshes/
>> metric fetching).
> 
> The usefulness of such a thing depends a bit on what we want to
> limit (amount of total requests to a target remote?), and why (reducing
> network traffic, load on target node, load on PDM, ...?)
> 
I think it makes sense to think about this from the following perspective:
It makes sense to limit concurrency (we already do), and if one limits
concurrency, I think that should be done globally, not per task.
Otherwise the total number of connections at a time might depended
on how polling tasks are scheduled (e.g. if they happen to run at the same
time or not, which might lead to hard to predict load spikes)

Later we could have separate settings for 'groups of remotes', e.g. to limit
the number of concurrent connections via some slow VPN tunnel to some data center that
houses a couple, but not all remotes.

>>
>> For actually controlling the concurrency we could maybe have a globally available
>> semaphore (potential deadlock potential in some cases).
>> Alternatively, we could think about having a 'background request queue' and
>> a 'background request scheduler', which does the actual requests on behalf of
>> the other tasks.
> 
> semaphores sound nice but IMO often aren't, especially as they lack
> introspection, I'm sure that's better in rust than in C, but  a dedicated
> queueing mechanism _might_ be nicer, especially as one can then also
> use more useful approaches to queue/schedule things. And, e.g., once our
> target APIs support something nice as QUIC we could even batch requests
> to a single target (remote) together.

After playing around with some ideas, I'll probably go with a semaphore at first, but
abstracted in a way that it should be quite easy to change to something more complex later.


> 
>>
>> I'll give this a bit more thoughts in the next days/weeks, so maybe don't merge
>> this in the meanwhile.
>>
> 
> ack.

I've sent a [v3] with some of the settings which would potentially be changed again
by a request queue/scheduler removed, namely:
  - max-concurrency
  - *-interval-offset
  - *-connection-delay

I'll be on vacation soon and I don't think I can whip out the request queue thingy
before that, and I didn't want to keep this patch series in a limbo state until I return :)
Hopefully v3 is ready to be applied and I can then build on that later.

[v3]: https://lore.proxmox.com/pdm-devel/20250416125642.291552-1-l.wagner@proxmox.com/T/#t

-- 
- Lukas



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


      reply	other threads:[~2025-04-16 13:03 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-14 13:06 [pdm-devel] " Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 01/28] test support: add NamedTempFile helper Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 02/28] test support: add NamedTempDir helper Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 03/28] pdm-api-types: add CollectionSettings type Lukas Wagner
2025-02-18 15:26   ` Wolfgang Bumiller
2025-02-18 15:31     ` Stefan Hanreich
2025-02-21  8:27     ` Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 04/28] pdm-config: add functions for reading/writing metric collection settings Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 05/28] metric collection: split top_entities split into separate module Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 06/28] metric collection: save metric data to RRD in separate task Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 07/28] metric collection: rework metric poll task Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 08/28] metric collection: persist state after metric collection Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 09/28] metric collection: skip if last_collection < MIN_COLLECTION_INTERVAL Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 10/28] metric collection: collect overdue metrics on startup/timer change Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 11/28] metric collection: add tests for the fetch_remotes function Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 12/28] metric collection: add test for fetch_overdue Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 13/28] metric collection: pass rrd cache instance as function parameter Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 14/28] metric collection: add test for rrd task Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 15/28] metric collection: wrap rrd_cache::Cache in a struct Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 16/28] metric collection: record remote response time in metric database Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 17/28] metric collection: save time needed for collection run to RRD Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 18/28] metric collection: periodically clean removed remotes from statefile Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 19/28] api: add endpoint for updating metric collection settings Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 20/28] api: add endpoint to trigger metric collection Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 21/28] api: remotes: trigger immediate metric collection for newly added nodes Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 22/28] api: add api for querying metric collection RRD data Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 23/28] api: metric-collection: add status endpoint Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 24/28] pdm-client: add metric collection API methods Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 25/28] cli: add commands for metric-collection settings, trigger, status Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 26/28] metric collection: factor out handle_tick and handle_control_message fns Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 27/28] metric collection: skip missed timer ticks Lukas Wagner
2025-02-14 13:06 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 28/28] metric collection: use JoinSet instead of joining from handles in a Vec Lukas Wagner
2025-02-21 13:19 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 00/28] metric collection improvements (concurrency, config, API, CLI) Maximiliano Sandoval
2025-03-14 14:10 ` Lukas Wagner
2025-03-16 21:45   ` Thomas Lamprecht
2025-04-16 13:03     ` Lukas Wagner [this message]

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=a5c2f0dc-1090-4f4f-aa0e-895b1f877a18@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal