public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH proxmox-datacenter-manager 00/15] change task cache mechanism from time-based to max-size FIFO
Date: Wed, 5 Feb 2025 16:34:30 +0100	[thread overview]
Message-ID: <fdd35d7f-8b0c-449c-9a6c-d4690de51786@proxmox.com> (raw)
In-Reply-To: <4d3871ae-6cbc-40e4-96e2-8aee37563f79@proxmox.com>

Am 31.01.25 um 10:35 schrieb Lukas Wagner:
> Some additional context as to explain the 'why', since @Thomas and @Wolfgang requested it:
> 
> The status quo for the task cache is to fetch a certain time-based range of tasks
> (iirc the last seven days) from every remote and cache this for a certain period
> of time (max-age). If the cached data is too old, we discard the task data
> and fetch the same time range again.
> My initial reasoning behind designing it like this was to keep the
> 'ground truth' completely on the remote side, so *if* somebody were to mess with

Yeah, I agree with Wolfgang here, nothing in our API allows messing with that,
neither does a first-class PVE CLI command or the like. If we want to hedge
against that we basically cannot cache anything at all, rendering them rather
useless, as even RRD metrics could be altered.

Using the start-time of the newest available task from the cache as (inclusive)
boundary to request newer tasks that happened since the last query would be
enough or?

Pruning older tasks from the cache after some max-time or max-size limit
is overstepped would then make sense though.

IMO it for purging older task log entries to avoid huge cache we should mainly
focus on the age of entries limit, e.g. most our dashboards will focus on showing
the task results since X hours, where a sensible minimum for X is probably at
least 24 hours, probably better more like 3 to 5 days to be able to (quickly)
see what happened over a weekend, maybe with some holiday attached to it.

A generously high size limit as additional upper bound to avoid running out of
space might still be nice to hedge against some node, user or api-tooling going
crazy and producing a huge amount of tasks.

> the task archive, we would be consistent after a refresh. Also this allowed to
> keep the caching logic on PDM side much simpler, since we not doing much more
> than caching the API response from the remote - the same way we already do it
> for resources, subscription status, etc.
> 
> The downside is we had some unnecessary traffic, since we keep on fetching old tasks
> that we already received.
> 
> I originally posted this as an RFC right before the holidays to get an initial approach
> out to get some feedback on the approach (I wasn't too sure myself if the original approach
> was a good idea) and also to somewhat unblock UI development for the remote task view.
>
> The RFC patches were applied relatively quickly; in a short conversation with Dietmar
> he mentioned that it might be better to just fetch the most recent missing tasks so
> that we don't have to retransmit the old data over and over again.

Yeah, I'm still not convinced that rushing such things through, especially
at RFC stage, will make overall development go quicker. Keeping communication
(solely) locked in isolated off-list discussion isn't helping for sure though.
Here both of that caused IMO rather more overhead.

> Some time later Dominik also approached me and suggested the approach that is implemented in
> this patch series. Which is
>   - instead of simply caching the remotes API response, we try to replicate
>     the task archive locally
>   - memorize the time when we last got the latest tasks and only request
>     what is missing since then
>   - limit the replicated task archive's size, dropping the oldest tasks
>     when the size is exceeded
> 
> I didn't have any objections so I went ahead and implemented it.
> 
> The main benefits is that we have to transmit much less data.

Yes, but the original idea would act the same in this regard if it did not
hedge against things that cannot really happen without manually messing around
(at which point all bets are off); or what am I missing here? To be fair: I
only skimmed the patches and asked Wolfgang a bit about it as he looked through
them more closely, so I might indeed miss something or just be slightly confused
about the presentation here.


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


  parent reply	other threads:[~2025-02-05 15:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28 12:25 Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 01/15] pdm-api-types: derive Debug and PartialEq for TaskListItem Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 02/15] test support: add NamedTempFile helper Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 03/15] task cache: add basic test for TaskCache Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 04/15] task cache: remove max-age machanism Lukas Wagner
2025-01-29 18:27   ` Thomas Lamprecht
2025-01-30  8:01     ` Lukas Wagner
2025-01-30 16:06       ` Thomas Lamprecht
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 05/15] task cache: add FIFO cache replacement policy Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 06/15] remote tasks: move to dir based module Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 07/15] task cache: move to its own submodule Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 08/15] task cache: fetch every 5mins, requesting only missing tasks Lukas Wagner
2025-01-31 13:42   ` Wolfgang Bumiller
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 09/15] remote tasks: return tasks in stable order Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 10/15] remote tasks: allow to force-fetch latest tasks Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 11/15] fake remote: add missing fields to make the debug feature compile again Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 12/15] fake remote: generate fake task data Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 13/15] task cache: tests: improve test coverage Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 14/15] remote tasks: fix unused variable warning Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 15/15] remote-tasks: restrict function visibility Lukas Wagner
2025-01-31  9:35 ` [pdm-devel] [PATCH proxmox-datacenter-manager 00/15] change task cache mechanism from time-based to max-size FIFO Lukas Wagner
2025-01-31 13:36   ` Wolfgang Bumiller
2025-01-31 13:51     ` Wolfgang Bumiller
2025-02-05 15:34   ` Thomas Lamprecht [this message]
2025-02-06 10:13     ` Lukas Wagner
2025-02-12  9:19       ` Thomas Lamprecht

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=fdd35d7f-8b0c-449c-9a6c-d4690de51786@proxmox.com \
    --to=t.lamprecht@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