From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0FF3C1FF14C for ; Fri, 15 May 2026 16:49:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 192A611901; Fri, 15 May 2026 16:49:35 +0200 (CEST) From: Lukas Wagner To: pdm-devel@lists.proxmox.com Subject: [PATCH datacenter-manager v3 3/4] api: resources: subscriptions: switch over to api_cache Date: Fri, 15 May 2026 16:49:26 +0200 Message-ID: <20260515144927.427114-4-l.wagner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260515144927.427114-1-l.wagner@proxmox.com> References: <20260515144927.427114-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778856561621 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: MPQBNIEEEFIXRDG6ANH3WY2Q23ZHHCUB X-Message-ID-Hash: MPQBNIEEEFIXRDG6ANH3WY2Q23ZHHCUB X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Instead of storing the subscription state in a module-level HashMap, use the newly introduced api_cache module to cache the status. Signed-off-by: Lukas Wagner --- Notes: Changes since the RFC: - Use new async cache interface Changes since v1: - Make cache self-healing in case an entry could not be read or deserialized. This ensure resiliency when the cached data type changes unexpectedly. In this case, we just log an error and return None Changes since v2: - don't update cache if somebody else has updated it in the meanwhile. Additionally, return the more up-to-date data from the cache, so that we always the freshest subscription data possible server/src/api/resources.rs | 88 ++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 49 deletions(-) diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs index 50315b11..8bcb8ce3 100644 --- a/server/src/api/resources.rs +++ b/server/src/api/resources.rs @@ -30,9 +30,10 @@ use proxmox_schema::{api, parse_boolean}; use proxmox_sortable_macro::sortable; use proxmox_subscription::SubscriptionStatus; use pve_api_types::{ClusterResource, ClusterResourceNetworkType, ClusterResourceType}; +use serde::{Deserialize, Serialize}; use crate::metric_collection::top_entities; -use crate::{connection, views}; +use crate::{api_cache, connection, views}; pub const ROUTER: Router = Router::new() .get(&list_subdirs_api_method!(SUBDIRS)) @@ -798,15 +799,11 @@ async fn get_top_entities( Ok(res) } -#[derive(Clone)] +#[derive(Clone, Serialize, Deserialize)] struct CachedSubscriptionState { node_info: HashMap>, - timestamp: i64, } -static SUBSCRIPTION_CACHE: LazyLock>> = - LazyLock::new(|| RwLock::new(HashMap::new())); - /// Get the subscription state for a given remote. /// /// If recent enough cached data is available, it is returned @@ -815,66 +812,59 @@ pub async fn get_subscription_info_for_remote( remote: &Remote, max_age: u64, ) -> Result>, Error> { - if let Some(cached_subscription) = get_cached_subscription_info(&remote.id, max_age) { + if let Some(cached_subscription) = get_cached_subscription_info(&remote.id, max_age).await? { Ok(cached_subscription.node_info) } else { let node_info = fetch_remote_subscription_info(remote).await?; let now = proxmox_time::epoch_i64(); - update_cached_subscription_info(&remote.id, &node_info, now); + + if let Some(existing_state) = + update_cached_subscription_info(&remote.id, node_info.clone(), now).await? + { + // Somebody else updated the cache while we performed the API request, + // return the more recent data instead of the data we just fetched. + return Ok(existing_state.node_info); + } Ok(node_info) } } -fn get_cached_subscription_info(remote: &str, max_age: u64) -> Option { - let cache = SUBSCRIPTION_CACHE - .read() - .expect("subscription mutex poisoned"); +const SUBSCRIPTION_STATE_CACHE_KEY: &str = "subscription-state"; - if max_age == 0 { - return None; - } - if let Some(cached_subscription) = cache.get(remote) { - let now = proxmox_time::epoch_i64(); - let diff = now - cached_subscription.timestamp; +async fn get_cached_subscription_info( + remote: &str, + max_age: u64, +) -> Result, Error> { + let cache = api_cache::read_remote(remote).await?; + let subscription_state = cache + .get_with_max_age(SUBSCRIPTION_STATE_CACHE_KEY, max_age as i64) + .await + .inspect_err(|err| log::error!("could not read subscription-state from API cache: {err}")) + .ok() + .flatten(); - if diff >= max_age as i64 || diff < 0 { - // value is too old or from the future - None - } else { - Some(cached_subscription.clone()) - } - } else { - None - } + Ok(subscription_state) } /// Update cached subscription data. /// -/// If the cache already contains more recent data we don't insert the passed resources. -fn update_cached_subscription_info( +/// If the cache already contains more recent data, this function returns the already +/// stored state as `Ok(Some(state))`. If the data that was passed in replaced the cache +/// entry, `Ok(None)` is returned. +async fn update_cached_subscription_info( remote: &str, - node_info: &HashMap>, + node_info: HashMap>, now: i64, -) { - // there is no good way to recover from this, so panicking should be fine - let mut cache = SUBSCRIPTION_CACHE - .write() - .expect("subscription mutex poisoned"); +) -> Result, Error> { + let cache = api_cache::write_remote(remote).await?; - if let Some(cached_resource) = cache.get(remote) { - // skip updating if the data is new enough - if cached_resource.timestamp >= now { - return; - } - } - - cache.insert( - remote.into(), - CachedSubscriptionState { - node_info: node_info.clone(), - timestamp: now, - }, - ); + Ok(cache + .set_if_newer_with_timestamp( + SUBSCRIPTION_STATE_CACHE_KEY, + CachedSubscriptionState { node_info }, + now, + ) + .await?) } /// Maps a list of node subscription infos into a single [`RemoteSubscriptionState`] -- 2.47.3